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

Support aliasing of built-in property names (for Serilog.Filters.Expressions migration) #67

Merged
merged 1 commit into from
Feb 14, 2022

Conversation

nblumhardt
Copy link
Member

@nblumhardt nblumhardt commented Feb 13, 2022

Phew! Sorry that took a while, @warrenbuckley :-) Hope you had a great weekend!

I think you're right about keeping the scope limited. Here's how the LegacyFilterSyntaxNameResolver would look, with this change (untested :-)):

public class LegacyLevelPropertyNameResolver: NameResolver
{
    public static LogEventPropertyValue? Has(LogEventPropertyValue? value)
    {
        return new ScalarValue(value != null);
    }
    
    public override bool TryResolveFunctionName(string name, [NotNullWhen(true)] out MethodInfo? implementation)
    {
        if ("Has".Equals(name, StringComparison.OrdinalIgnoreCase))
        {
            implementation = GetType().GetMethod("Has", BindingFlags.Static | BindingFlags.Public)!;
            return true;
        }

        implementation = null;
        return false;
    }

    public override bool TryResolveBuiltInPropertyName(string alias, [NotNullWhen(true)] out string? target)
    {
        target = alias switch
        {
            "Exception" => "x",
            "Level" => "l",
            "Message" => "m",
            "MessageTemplate" => "mt",
            "Properties" => "p",
            "Timestamp" => "t",
            _ => null
        };
        
        return target != null;
    }
}

Fixes #63
Fixes #64

@nblumhardt
Copy link
Member Author

(Requiring that the target Has function is public is a bit of a flaw in NameResolver, might think about offering an alternative (e.g. lambda) option, one of these days 🤔)

actual = ConstantArrayEvaluator.Evaluate(actual);
actual = WildcardComprehensionTransformer.Expand(actual);
actual = ConstantArrayEvaluator.Rewrite(actual);
actual = WildcardComprehensionTransformer.Rewrite(actual);
Copy link
Member Author

Choose a reason for hiding this comment

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

A little more naming consistency :-)

@warrenbuckley
Copy link
Contributor

warrenbuckley commented Feb 14, 2022

Looks good to me @nblumhardt 👍
The new TryResolveBuiltInPropertyName method to override will solve this problem and again once this PR is done then the readme can be updated to show this example

Keeping my eyes peeled 👀 for when I can use an updated release

@nblumhardt
Copy link
Member Author

👍 - 3.2.2-dev-00083 is now on NuGet, if this does the trick for you I'll queue up a release build (which should probably be 3.3.0, since the added feature warrants a minor version bump.)

@warrenbuckley
Copy link
Contributor

Hiya @nblumhardt
I have updated a draft Umbraco PR and a draft PR to the electron desktop app and both are working fine with the updated 3.2.2-dev-00083 build.

Are you able to notify me please when 3.3.0 is released so I can finalize these PRs 😄

@nblumhardt nblumhardt mentioned this pull request Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants