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

Caching Issue in Literals Dictionary Causes Valid Expression to Fail #614

Closed
RenanCarlosPereira opened this issue Jun 25, 2024 · 13 comments
Closed

Comments

@RenanCarlosPereira
Copy link

Description:
There is a bug in the Rules Engine's RuleExpressionParser where a valid expression fails due to a caching issue in the Literals dictionary in System.Linq.Dynamic.Core.

Steps to Reproduce:

  1. Create a test case where a nullable decimal property is evaluated against a double literal.
  2. The first expression (Board.NumberOfMembers = 0.2d) throws an exception as expected due to a type mismatch.
  3. The second expression (Board.NumberOfMembers = 0.2) should be valid but throws an exception because the first expression is cached in the Literals dictionary.

Test Code:

[Fact]
public void Test()
{
    var board = new { NumberOfMembers = default(decimal?) };

    var parameter = RulesEngine.Models.RuleParameter.Create("Board", board);

    var parser = new RulesEngine.ExpressionBuilders.RuleExpressionParser();

    try
    {
        const string expression1 = "Board.NumberOfMembers = 0.2d";
        var result1 = parser.Evaluate<bool>(expression1, new[] { parameter });
        result1.Should().BeFalse();
    }
    catch (Exception)
    {
        // passing it over.
    }

    // This will throw an exception even if the expression is valid,
    // because the first one executed and cached in the Literals Dictionary.
    // This should not throw as it's valid.
    const string expression2 = "Board.NumberOfMembers = 0.2";
    var result2 = parser.Evaluate<bool>(expression2, new[] { parameter });
    result2.Should().BeFalse();
}

Expected Behavior:
The second expression (Board.NumberOfMembers = 0.2) should be evaluated correctly and not throw an exception.

Actual Behavior:
The second expression throws an exception due to the first expression being cached in the Literals dictionary.

Potential Cause:
The caching mechanism in the Literals dictionary in System.Linq.Dynamic.Core does not handle different numeric literal types correctly, leading to invalid caching of expressions.

@asulwer
Copy link

asulwer commented Jun 25, 2024

this repo is no longer maintained. this issue will always stay open. please use a maintained fork, such as mine, to submit a new issue

i did see that your issue was resolved?

once a new version from zzzprojects is released then i will update my fork. i am adding this test to RulesEngine to support the future solution

@asulwer
Copy link

asulwer commented Jun 25, 2024

when i flip the two expressions the issue doesn't appear. maybe a temp solution?

@RenanCarlosPereira
Copy link
Author

yeah, the issue is not showing up when you change the order because it is caching the first one.
So once the dictionary has the value without the literal it will work.
but that can't be a temporary solution because when I spin up my app the dictionary is empty, so if someone creates a rule passing that literal it will be cached and the only thing that I can do is restart the application.

I opened the bug here because the Nuget is pointing to this repo, how are we managing the nuget versions?

Do we have access to keep moving the Nuget from your fork?

@asulwer
Copy link

asulwer commented Jun 25, 2024

The Nuget solution will be an entirely new package. I am working on solving the many issues that exist before i release to Nuget. You can manually add the release from github until i release a Nuget solution?

i am working on a possible solution (as i type this) while we wait on zzzprojects.

@asulwer
Copy link

asulwer commented Jun 25, 2024

the second expression works IF you use literal notation. i presented this exact solution in a previous issue you opened 583?

const string expression2 = "Board.NumberOfMembers = 0.2m";

first one fails as expected and second one does not

This 'may' be a caching issue with Sytem.Linq.Dynamic.Core, as you say. based on this discussion the issue is your use of literal notation. the second fails because the literal notation is not correct so it falls back on what was previously used.

Note: 'falling back' is my educated guess as i have not looked at Ssytem.Linq.Dynamic.Core code to confirm this.

@RenanCarlosPereira
Copy link
Author

I opened a PR there to fix this, that got merged to master today a few hours ago.

They just need to release a new version.

Not sure how you will do a workaround in this, cuz I checked the code and the expressions are created internally in the package

zzzprojects/System.Linq.Dynamic.Core#824

@RenanCarlosPereira
Copy link
Author

This other bug you mentioned is not related to this one

The 583 is evaluating the expression wrong because of fast compiler

@asulwer
Copy link

asulwer commented Jun 25, 2024

the other bug is a symptom of the same underlying issue, literal notation

@RenanCarlosPereira
Copy link
Author

RenanCarlosPereira commented Jun 25, 2024

I don't think it is related man, the issue issue is related to the lib https://github.com/dadhi/FastExpressionCompiler

We can talk about it in that thread and leave this one.
As this one will get resolved when we got a new version of dynamic linq

Check out this line

if(_reSettings.UseFastExpressionCompiler)

Thats the problem with the fast compiler, thats why its not evaluating correctly.
It might be a bug inside that other pack

What do you think?

@asulwer
Copy link

asulwer commented Jun 25, 2024

as to that other issue, i have that solved in my fork. i pushed a nuget package RulesEngineEx

@RenanCarlosPereira
Copy link
Author

Nice, curious on how you fixed it, do you have an PR? 😄

@asulwer
Copy link

asulwer commented Jun 25, 2024

this project is dead at this location, the author stated so. he is no longer working for Microsoft and they have abandoned this project. further development is being done on my fork.

@RenanCarlosPereira
Copy link
Author

RenanCarlosPereira commented Jun 30, 2024

this bug was resolved in the lib system.dynamic.linq.core in this PR:
zzzprojects/System.Linq.Dynamic.Core#824

there's a new version of this package that got released, the rules engine got updated with this package and the tests are passing now.

this was resolved in this fork:
https://github.com/asulwer/RulesEngine

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

2 participants