System.Linq.Dynamic.Core icon indicating copy to clipboard operation
System.Linq.Dynamic.Core copied to clipboard

Dynamic linq not correctly apply on list of DynamicClass

Open anc-dev-lk opened this issue 3 years ago • 8 comments

I have following code. From external source I'm getting field.Name = "firstName" and field.Value = "firstValue".

var dynamicIndexes = new List<DynamicClass>();

var props = new DynamicProperty[]
{
   new DynamicProperty(field.Name, typeof(string))
};

Type type = DynamicClassFactory.CreateType(props);

var dynamicClass = Activator.CreateInstance(type) as DynamicClass;
dynamicClass.SetDynamicPropertyValue<string>(field.Name, field.Value);

dynamicIndexes.Add(dynamicClass);

var query = dynamicIndexes.AsQueryable();
bool isValid = query.Any("firstName eq \"firstValue\"");

But I'm getting isValid as false.

I did following tests to understand the issue:

added following code line at the beginning, to overwrite the value and then I'm getting isValid as true.

       field.Value = "firstValue";

but if I change that as follows then I'm getting isValid as false.

       byte[] utfBytes = Encoding.Default.GetBytes("firstValue");
       field.Value = Encoding.Default.GetString(utfBytes);

I'm guessing dynamic linq not working with string that has different encodings.

anc-dev-lk avatar Jun 18 '22 20:06 anc-dev-lk

It's a bug, as far as i can tell, but it is not related to encodings.

The bug rather seems to be related to strings being interned (which string literals defined in the source code of the program typically are) vs. strings that are not interned.

For reasons i don't know, the expression parser/language of dynamic linq does not like strings that are not interned (i.e., strings that are dynamically/programmatically created during program execution). And it so happens that Encoding.Default.GetString(utfBytes) creates a new string that is not interned.

You should be able to reproduce the same bug effect without involving anything encoding-related at all by replacing Encoding.Default.GetBytes/Encoding.Default.GetString with something like:

var cc = "firstValue".ToCharArray();
field.Value = new string(cc);

(If you can reproduce the problem with the char array trick shown here, it would be nice if you could edit the title of your issue report and remove the mention of encodings from it so the project maintainers don't get send on the wrong track wrt to the underlying issue...)

elgonzo avatar Jun 18 '22 20:06 elgonzo

Some further insights about what's going on here...

The System.Linq.Dynamic.Core library is building an System.Expressions.Expression tree from the string expression in query.Any("firstName eq \"firstValue\""). The left-hand expression is the identifier firstName. So far so good.

The System.Linq.Dynamic.Core library tries to figure out the type of the left-hand expression (which is being just the identifier firstName. Since query is a List<DynamicClass>, it will reflect upon the type DynamicClass to find a field or property with the name firstName to figure out its type. But, of course, DynamicClass itself does not feature any firstName member, so System.Linq.Dynamic.Core is treating firstName as being of type object.

And treating firstName as being of type object is the crux of the problem.

The behavior emanates from the System.Linq.Expressions classes (provided by the .NET base class library). It can be reproduced with some simple System.Linq.Expressions.Expression setup without even involving System.Linq.Dynamic.Core:

// lets first create two non-interned strings (i.e., not going to use string literals)
// Both strings have the same value, but since we don't use string literals, each string is a separate instance.

var s1 = new string(new char[] { 'f', 'i', 'r', 's', 't', 'V', 'a', 'l', 'u', 'e' });
var s2 = new string(new char[] { 'f', 'i', 'r', 's', 't', 'V', 'a', 'l', 'u', 'e' });

// Create left-hand and right-hand constant expressions, with the values of the strings s1 and s2.
// However, the left-hand expression will be of type object, whereas the right-hand expression will be of type string.

var left = System.Linq.Expressions.Expression.Constant(s1, typeof(object));
var right = System.Linq.Expressions.Expression.Constant(s2, typeof(string));

// now create the equality expression 'left equals right'

var eq = System.Linq.Expressions.Expression.Equal(left, right);

// and evaluate the equality expression

var lam = System.Linq.Expressions.Expression.Lambda(eq);
var fn = (Func<bool>) lam.Compile();
bool result = fn();
Console.WriteLine(result);

Dotnetfiddle: https://dotnetfiddle.net/IArvJA

This will result in the expression evaluating to false, even though both the left-hand and right-hand expression having exactly the same value. It's just that the left-hand expression has been typed as object, not as string. Kablooey!

(It really is the left-hand expression being typed as object. If it were typed as string, the result would be, as expected, true. https://dotnetfiddle.net/5VhuVF)

elgonzo avatar Jun 19 '22 16:06 elgonzo

Knowing this, a workaround is possible:

bool isValid = query.Any("firstName.ToString() eq \"firstValue\"");

Since basically any type derives from System.Object (except interfaces and some such, perhaps, i don't know really...), the ToString method (declared by System.Object) can be used to make the type of the left-hand expression here to be string (which is practical in this scenario here, since firstName contains a string value anyways), thus avoiding the issue. But i admit myself, it's not a pretty workaround...

elgonzo avatar Jun 19 '22 17:06 elgonzo

Knowing this, a workaround is possible:

bool isValid = query.Any("firstName.ToString() eq \"firstValue\"");

Since basically any type derives from System.Object (except interfaces and some such, perhaps, i don't know really...), the ToString method (declared by System.Object) can be used to make the type of the left-hand expression here to be string (which is practical in this scenario here, since firstName contains a string value anyways), thus avoiding the issue. But i admit myself, it's not a pretty workaround...

Thank you very much for your clear explanation. but this workaround not work for me because query expression ("firstName eq "firstValue"") capturing from external source and that can be more complex than the example. I have to use some tokenizer for identify properties and then I can use simple linq query without using dynamic linq.

anc-dev-lk avatar Jun 20 '22 04:06 anc-dev-lk

Some further insights about what's going on here...

The System.Linq.Dynamic.Core library is building an System.Expressions.Expression tree from the string expression in query.Any("firstName eq \"firstValue\""). The left-hand expression is the identifier firstName. So far so good.

The System.Linq.Dynamic.Core library tries to figure out the type of the left-hand expression (which is being just the identifier firstName. Since query is a List<DynamicClass>, it will reflect upon the type DynamicClass to find a field or property with the name firstName to figure out its type. But, of course, DynamicClass itself does not feature any firstName member, so System.Linq.Dynamic.Core is treating firstName as being of type object.

And treating firstName as being of type object is the crux of the problem.

The behavior emanates from the System.Linq.Expressions classes (provided by the .NET base class library). It can be reproduced with some simple System.Linq.Expressions.Expression setup without even involving System.Linq.Dynamic.Core:

// lets first create two non-interned strings (i.e., not going to use string literals)
// Both strings have the same value, but since we don't use string literals, each string is a separate instance.

var s1 = new string(new char[] { 'f', 'i', 'r', 's', 't', 'V', 'a', 'l', 'u', 'e' });
var s2 = new string(new char[] { 'f', 'i', 'r', 's', 't', 'V', 'a', 'l', 'u', 'e' });

// Create left-hand and right-hand constant expressions, with the values of the strings s1 and s2.
// However, the left-hand expression will be of type object, whereas the right-hand expression will be of type string.

var left = System.Linq.Expressions.Expression.Constant(s1, typeof(object));
var right = System.Linq.Expressions.Expression.Constant(s2, typeof(string));

// now create the equality expression 'left equals right'

var eq = System.Linq.Expressions.Expression.Equal(left, right);

// and evaluate the equality expression

var lam = System.Linq.Expressions.Expression.Lambda(eq);
var fn = (Func<bool>) lam.Compile();
bool result = fn();
Console.WriteLine(result);

Dotnetfiddle: https://dotnetfiddle.net/IArvJA

This will result in the expression evaluating to false, even though both the left-hand and right-hand expression having exactly the same value. It's just that the left-hand expression has been typed as object, not as string. Kablooey!

(It really is the left-hand expression being typed as object. If it were typed as string, the result would be, as expected, true. https://dotnetfiddle.net/5VhuVF)

Thanks for the explanation. but @elgonzo when I overwrite field.Value = "firstValue"; at the beginning, this is work correctly. how you can explain that?

anc-dev-lk avatar Jun 20 '22 05:06 anc-dev-lk

Thanks for the explanation. but @elgonzo when I overwrite field.Value = "firstValue"; at the beginning, this is work correctly. how you can explain that?

Because you are using a string literal here. String literals are by default interned.

If you are curious of how exactly this behavior materializes on a more technical level: Two aspects of System.Linq.Expressions conspire here.

The first factor is that the eq operator is relying on System.Linq.Expressions.BinaryExpression. When comparing two exrepssions that are typed as reference types with System.Linq.Expressions.BinaryExpression, and one of the expressions (perhaps specifically the left-hand side expression only) is of type object, then the equality comparison is pretty much reduced to a simple reference comparison (similar to object.ReferenceEquals(o1,o2))).

The second factor is that for the "firstValue" string in firstName eq "firstValue", the library uses a System.Linq.Expressions.ConstantExpression. And a ConstantExpression of type string will intern the string provided to it (specifically, it will intern its string value when it is being compiled as part of a LambdaExpression, the latter being necessary to actually evaluate/execute the expression).

So basically, the library turns firstName eq "firstValue" into a System.Linq.Expressions representation where the right-hand side is a ConstantExpression, interning its string value. That interned string instance then is reference-compared (because the left-hand expression is of type object) with whatever the left-hand side provides. Thus, in the end, the reference comparison can only have a chance to result in true if the left-hand side provides also an interned(!) string instance.

I only got to know this myself after asking about this behavior on dotnet's own github discussion section here: https://github.com/dotnet/runtime/discussions/70962

elgonzo avatar Jun 20 '22 08:06 elgonzo

@anc-dev-lk Is your question answered?

StefH avatar Aug 02 '22 19:08 StefH

@StefH It is a bug. do you have any plan to fix that?

anc-dev-lk avatar Aug 09 '22 10:08 anc-dev-lk

Hello @elgonzo and @anc-dev-lk ,

I tried to reproduce your issue with this unit test, but I could not reproduce it.

[Fact]
public void DynamicClassArray()
{
    // Arrange
    var field = new
    {
        Name = "firstName",
        Value = "firstValue"
    };
    var dynamicClasses = new List<DynamicClass>();

    var props = new DynamicProperty[]
    {
        new DynamicProperty(field.Name, typeof(string))
    };

    var type = DynamicClassFactory.CreateType(props);

    var dynamicClass = (DynamicClass) Activator.CreateInstance(type);
    dynamicClass.SetDynamicPropertyValue(field.Name, field.Value);

    dynamicClasses.Add(dynamicClass);

    var query = dynamicClasses.AsQueryable();

    // Act
    var isValid = query.Any("firstName eq \"firstValue\"");

    // Assert
    isValid.Should().BeTrue();
}

Can you please help me building a unit test that fails?

StefH avatar Mar 03 '23 19:03 StefH

@StefH

I tried to reproduce your issue with this unit test, but I could not reproduce it.

That is because the left-hand side of the query (the firstName identifier) in the test method uses an interned string as its value, hence why it fails to reproduce the issue. The value is an interned string in the test because the value specified is a string literal (Value = "firstValue").

To reproduce the issue, you will need to avoid using string literals here, as string literals are interned by default. One way to get around this for the test is using one of the string constructors to dynamically create a string, or use any other string function that produces a new string instance dynamically (such as string.Concat, string.Substring, etc...).

So, instead of Value = "firstValue", try doing either of this:

    // Arrange
    var cc = "firstValue".ToCharArray();
    var field = new
    {
        Name = "firstName",
        Value =  new string(cc)
    };

or

    // Arrange
    var field = new
    {
        Name = "firstName",
        Value =  string.Concat("first", "Value")
    };

or any other code construct that avoids creating an interned string...

dotnetfiddle showing the test failing when using a non-interned string for field.Value: https://dotnetfiddle.net/DHvT1f

elgonzo avatar Mar 03 '23 20:03 elgonzo

Hello @anc-dev-lk and @elgonzo ,

I can reproduce the issue using that example code and you analysis is correct.

Another part of the issue is this code:

var dynamicClass = (DynamicClass) Activator.CreateInstance(type);

Because the result from CreateInstance (which is actually another type : <>f__AnonymousType0`1[System.String]), the information about the real properties, a string property, is lost.


If your use case is this: I have following code. From external source I'm getting field.Name = "firstName" and field.Value = "firstValue".

The solution for your issue is just passing the data (field), create a list of field entities and query on the "Value", like this:

        var cc = "firstValue".ToCharArray();
        var field = new
        {
            Name = "firstName",
            Value = new string(cc)
        };

        var array = new[] { field }.AsQueryable();

        var isValid = array.Select("it").Any("Value eq \"firstValue\"");

        isValid.Should().BeTrue();

https://github.com/zzzprojects/System.Linq.Dynamic.Core/pull/679

StefH avatar Mar 05 '23 12:03 StefH

@StefH

Another part of the issue is this code: var dynamicClass = (DynamicClass) Activator.CreateInstance(type); Because the result from CreateInstance (which is actually another type : <>f__AnonymousType0`1[System.String]), the information about the real properties, a string property, is lost.

No, that's incorrect. The type information of the firstName property is not lost in the created instance of the anonymous type. The created anonymous type has a public firstName property of type string, which can also be reflected upon. Dotnetfiddle: https://dotnetfiddle.net/ZeNtWa.


As i mentioned in my comment above the problem is related to using the type List<DynamicClass> as queryable source. To demonstrate and prove this, look at this dotnetfiddle: https://dotnetfiddle.net/dKC5XH. It will yield true for isValid. It still uses var dynamicClass = Activator.CreateInstance(type) as DynamicClass;, so this line itself is not an issue.

Rather, instead of using a List<DynamicClass>, the dotnetfiddle creates a List<T> of the type of the dynamic instance and adds this instance to the list. (It's not pretty because it required reflection to create such a list, though). As the dotnetfiddle demonstrates, avoiding List<DynamicClass> avoids the problem. (That said, i do not recommend this reflection-based approach, the dotnetfiddle is just to prove that the problem is not with var dynamicClass = (DynamicClass) Activator.CreateInstance(type);)

But, of course, var dynamicClass = Activator.CreateInstance(type) as DynamicClass; is practically a stepping stone leading to the problemtic code using List<DynamicClass>, because how could one create a queryable of the dynamically created type without resorting to "insane" reflection shenanigans...

elgonzo avatar Mar 05 '23 16:03 elgonzo

@elgonzo Thank you for the explanation.

The problem is this example for 'Any' (using List<DynamicClass>) is that the Type, in the Any method, is retrieved using the ElementType which is DynamicClass.

A solution for this is that the real type is provided as parameter to the Any method.

Or getting the first element from the IQuerable and call GetType(). However I am not sure this can work will a real IQueryable which accesses a DB.

However the question is if this a real problem in an application or a more technical -what if- issue.

StefH avatar Mar 05 '23 18:03 StefH

Hello @anc-dev-lk and @elgonzo,

There are several solutions for this issue:

  1. Passing the data (field), create a list of field entities and query on the "Value".

  2. Use a list from the real type:

var customListType = typeof(List<>).MakeGenericType(dynamicInstance.GetType());
  1. Use ToString()
var isValid = query.Any("firstName.ToString() eq \"firstValue\"");
  1. Use a cast to string:
var isValid = query.Any("string(firstName) eq \"firstValue\"");

StefH avatar Mar 10 '23 17:03 StefH