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

Add support for delegates values in StringArgumentValue #259

Closed
almostchristian opened this issue Apr 16, 2021 · 2 comments
Closed

Add support for delegates values in StringArgumentValue #259

almostchristian opened this issue Apr 16, 2021 · 2 comments

Comments

@almostchristian
Copy link
Contributor

Some sinks contain configuration properties/arguments that are delegate types (Action, Func).

The StringArgumentValue class already supports static field and property accessors for non-delegate types, and can be extended to support delegate values that are accessed from static properties, static fields or static non-generic methods. If the argument value points to a method, overload resolution should be done to match the target delegate with the reflected method.

A disadvantage could be that this would encourage the creation of static methods which some view as bad for testability.

Below is my proposed change:

if (TryParseStaticMemberAccessor(argumentValue, out var accessorTypeName, out var memberName))
{
    var accessorType = Type.GetType(accessorTypeName, throwOnError: true);
    // if target type is a delegate, look for a public static non-generic method in the accessor type
    if (typeof(Delegate).IsAssignableFrom(toType))
    {
        var methodCandidates = accessorType.GetTypeInfo().DeclaredMethods
            .Where(x => x.Name == memberName)
            .Where(x => x.IsPublic)
            .Where(x => !x.IsGenericMethod)
            .Where(x => x.IsStatic)
            .ToList();
    
        // if more than 1 candidates, get target type signature and perform overload resolution on candidate methods
        if (methodCandidates.Count > 1)
        {
            var delegateSig = toType.GetMethod("Invoke");
            var delegateParameters = delegateSig.GetParameters().Select(x => x.ParameterType);
            methodCandidates = methodCandidates
                .Where(x => x.ReturnType == delegateSig.ReturnType)
                .Where(x => x.GetParameters().Select(p => p.ParameterType).SequenceEqual(delegateParameters))
                .ToList();
        }
    
        var method = methodCandidates.SingleOrDefault();
  
        if (method != null)
        {
            return method.CreateDelegate(toType);
        }
@alexbrina
Copy link

I guess this would solve my issue with Elasticsearch sink, in which I need to set a Func<,> delegate to option ModifyConnectionSettings but I couldn't find a way to do this.

@Feofilakt
Copy link

Great, do it!

almostchristian added a commit to almostchristian/serilog-settings-configuration that referenced this issue Feb 24, 2022
nblumhardt added a commit that referenced this issue Sep 13, 2022
…types

Add support for delegate type values through StringArgumentValue #259
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants