Fluid filter in .NET Standard 2 lib receives a `System.Version` as a string when rendered in .NET 6

Question

Fluid filter in .NET Standard 2 lib receives a `System.Version` as a string when rendered in .NET 6

Closed this issue 2 years ago · 3 comments

Hi,

I have a .NET Standard 2.0 library with some Fluid helpers (I'm using Fluid.Core v2.2.15). In particular I have a filter that formats a System.Version object.

Here is the relevant library code:

public static class FluidHelpers
{
    public const string FormatVersionFilterName = "format_version";

    public static async ValueTask<string> ReplacePlaceholdersAsync(string text, IDictionary<string, object> values)
    {
        var templateOptions =
            new TemplateOptions
            {
                Trimming = TrimmingFlags.TagRight
            };

        templateOptions.Filters.AddFilter(FormatVersionFilterName, FormatVersion);

        var context = new TemplateContext(templateOptions);

        foreach (var item in values ?? new Dictionary<string, object>())
        {
            context.SetValue(item.Key, item.Value);
        }
        var parser = new FluidParser(new FluidParserOptions());
        parser.TryParse(text, out var template, out var error);
        return await template.RenderAsync(context);
    }

    private static ValueTask<FluidValue> FormatVersion(FluidValue input, FilterArguments arguments, TemplateContext context)
    {
        if (input.Type != FluidValues.Object || input.IsNil())
        {
            return new ValueTask<FluidValue>(input);
        }

        Version? version = input.ToObjectValue() as Version;

        if (version is null)
        {
            return new ValueTask<FluidValue>(input);
        }

        int fieldCount = -1;

        const string fieldCountParamName = "fieldCount";
        if (arguments.HasNamed(fieldCountParamName))
        {
            fieldCount = Convert.ToInt32(arguments[fieldCountParamName].ToNumberValue());
        }
        else if (arguments.Count > 0)
        {
            fieldCount = Convert.ToInt32(arguments.At(0).ToNumberValue());
        }

        string formattedVersion = string.Empty;

        if (fieldCount == -1)
        {
            formattedVersion = version.ToString();
        }
        else
        {
            try
            {
                formattedVersion = version.ToString(fieldCount);
            }
            catch
            {
            }
        }

        return new ValueTask<FluidValue>(new StringValue(formattedVersion));
    }
}

Then consider the following xUnit code:

[Theory]
[InlineData("{{ScriptVersion}}-{{AppVersion}}", "1.1.0-2.4.1.3")]
[InlineData("{{ScriptVersion | format_version: 2}}-{{AppVersion | format_version: fieldCount:  3}}", "1.1-2.4.1")]
[InlineData("{{ScriptVersion | format_version: 1}}-{{AppVersion | format_version: 6}}", "1-")]
[InlineData("{{ScriptVersion}}-{{AppVersion | format_version: 3}}", "1.1.0-2.4.1")]
public async Task VersionTest(string template, string expected)
{
    Dictionary<string, object> values =
        new Dictionary<string, object>
        {
            {"ScriptVersion", new Version(1, 1, 0)},
            {"AppVersion", new Version(2, 4, 1, 3)}
        };

    var result = await FluidHelpers.ReplacePlaceholdersAsync(template, values);
    result.Should().Be(expected);
}

If I run the tests in .NET 5 they are ok. If I run them in .NET 6 then they fail.

When I debug my filter code using .NET 5, the type of the input (input.Type) is an object , so the first if statement is false and the remaining code is executed producing the expected result:

        if (input.Type != FluidValues.Object || input.IsNil())
        {
            return new ValueTask<FluidValue>(input);
        }
        ...

When I use .NET 6 the type of the input is FluidValues.String so it immediately returns.

At the moment I use a workaround that checks if the input is an object or a string and then tries to parse it as a Version.

I wonder if I'm doing something wrong or if it's effectively a bug.

Thank you.

Answer 1 · 2022-08-10T16:58:49.000Z

Looking into it

In the meantime, an advice: options and parser are thread-safe and should be reused. In your code they are created on each invocation. Here is an idea on how to make it faster and less memory hungry.

public class FluidHelpers
{
    private static readonly FluidParser _parser;

    static FluidHelpers()
    {
        var templateOptions =
            new TemplateOptions
            {
                Trimming = TrimmingFlags.TagRight
            };

        templateOptions.Filters.AddFilter(FormatVersionFilterName, FormatVersion);

        _parser = new FluidParser(new FluidParserOptions());
    }

    public static async ValueTask<string> ReplacePlaceholdersAsync(string text, IDictionary<string, object> values)
    {
        var context = new TemplateContext(templateOptions);

        if (values != null)
        {
            foreach (var item in values)
            {
                context.SetValue(item.Key, item.Value);
            }
        }
        _parser.TryParse(text, out var template, out var error);
        return await template.RenderAsync(context);
    }
...

If the same text source is passed several times then you could even cache the parsed templates, at least with some time limits (using IMemoryCache for instance).

Answer 2 · 2022-08-10T17:24:41.000Z

The reason is because in .NET 6 Version got the IFormattable (actually ISpanFormattable) interface added. In Fluid an object that implements this interface is considered a string such that {{ value }} can output something meaningful.

This is a breaking change from .NET 5 to .NET 6.

There are multiple ways to fix it. For instance, with a custom converter (check doc in README), that will intercept a Version object and return a custom class. You could also parse the string in your filter (if it's a FluidValues.String) before continuing with the filter.

Answer 3 · 2022-08-10T18:24:16.000Z

Hi,

thank you very much for the quick and accurate reply.

I was missing the new interface implementation in the .NET 6 System.Version so now the behaviour is clear. So I can leave the solution that I had already adopted:

public static ValueTask<FluidValue> FormatVersion(FluidValue input, FilterArguments arguments, TemplateContext context)
{
    if ((input.Type != FluidValues.Object && input.Type != FluidValues.String) || input.IsNil())
    {
        return new ValueTask<FluidValue>(input);
    }

    Version? version = input.ToObjectValue() as Version;

    if (version is null && !Version.TryParse(input.ToStringValue(), out version))
    {
        return new ValueTask<FluidValue>(input);
    }
    
   //  rest of the code
   ...
}

Regarding your cache suggestion I actually use it already in my library, but I have extrapolated the important code removing other dependencies to avoid complicating the exposition of the problem. Thank you any way for your advice.