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

Dynamic linq not correctly apply on list of DynamicClass #593

Closed
anc-dev-lk opened this issue Jun 18, 2022 · 14 comments
Closed

Dynamic linq not correctly apply on list of DynamicClass #593

anc-dev-lk opened this issue Jun 18, 2022 · 14 comments
Assignees
Labels

Comments

@anc-dev-lk
Copy link

anc-dev-lk commented Jun 18, 2022

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 anc-dev-lk changed the title Dynamic linq not correctly apply on DynamicClass list Dynamic linq not correctly apply on strings with different encodings Jun 18, 2022
@elgonzo
Copy link

elgonzo commented Jun 18, 2022

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
Copy link

elgonzo commented Jun 19, 2022

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
Copy link

elgonzo commented Jun 19, 2022

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...

@anc-dev-lk anc-dev-lk changed the title Dynamic linq not correctly apply on strings with different encodings Dynamic linq not correctly apply on list of DynamicClass Jun 20, 2022
@anc-dev-lk
Copy link
Author

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
Copy link
Author

anc-dev-lk commented Jun 20, 2022

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?

@elgonzo
Copy link

elgonzo commented Jun 20, 2022

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: dotnet/runtime#70962

@StefH
Copy link
Collaborator

StefH commented Aug 2, 2022

@anc-dev-lk
Is your question answered?

@StefH StefH added the question label Aug 2, 2022
@anc-dev-lk
Copy link
Author

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

@StefH
Copy link
Collaborator

StefH commented Mar 3, 2023

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?

@elgonzo
Copy link

elgonzo commented Mar 3, 2023

@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

@StefH StefH self-assigned this Mar 5, 2023
@StefH
Copy link
Collaborator

StefH commented Mar 5, 2023

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();

#679

@elgonzo
Copy link

elgonzo commented Mar 5, 2023

@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...

@StefH
Copy link
Collaborator

StefH commented Mar 5, 2023

@elgonzo
Thank you for the explanation.

The problem is this example for 'Any' (using List) 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
Copy link
Collaborator

StefH commented Mar 10, 2023

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 StefH closed this as completed Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants