Calling Select on an existing AsyncEnumerable/Queryable throws ArgumentException
Closed this issue · 6 comments
Creating an issue because I'm unsure whether the fix I have in mind would actually be feasible.
When creating a mocked DbContext and Selecting a single property after applying a Where clause, the underlying construction of FakeEntityType fails with an ArgumentException. This is because Select calls CreateQuery with a TResult type which could potentially be anything.
Also see curtisy1@34e27aa for a working reproducible test case.
Stack-Trace:
System.ArgumentException : The specified type 'System.Guid' must be a non-interface reference type to be used as an entity type.
at Microsoft.EntityFrameworkCore.Metadata.Internal.EntityType..ctor(Type type, Model model, ConfigurationSource configurationSource)
at EntityFrameworkCore.Testing.Common.FakeEntityType..ctor(Type type) in C:\Users\user\source\repos\EntityFrameworkCore.Testing\src\EntityFrameworkCore.Testing.Common\FakeEntityType.cs:line 8
at EntityFrameworkCore.Testing.Common.AsyncEnumerable`1..ctor(IEnumerable`1 enumerable) in C:\Users\user\source\repos\EntityFrameworkCore.Testing\src\EntityFrameworkCore.Testing.Common\AsyncEnumerable.cs:line 27
at EntityFrameworkCore.Testing.Common.AsyncQueryProvider`1.CreateQuery[TElement](Expression expression) in C:\Users\user\source\repos\EntityFrameworkCore.Testing\src\EntityFrameworkCore.Testing.Common\AsyncQueryProvider.cs:line 70
at System.Linq.Queryable.Select[TSource,TResult](IQueryable`1 source, Expression`1 selector)
at EntityFrameworkCore.Testing.Common.Tests.BaseForQueryableTests`1.WhereWithSelect_Condition_ReturnsIdsThatSatisfyCondition() in C:\Users\user\source\repos\EntityFrameworkCore.Testing\src\EntityFrameworkCore.Testing.Common.Tests\BaseForQueryableTests.cs:line 923
The idea I had in mind is to also include MethodCallExpressions when checking whether the _innerEnumerable is an IQueryable<T> but this somehow feels wrong
AsyncEnumerable.cs:21
if (_innerEnumerable is IQueryable<T> queryable && (queryable.Expression is Microsoft.EntityFrameworkCore.Query.QueryRootExpression || queryable.Expression is MethodCallExpression))
Another way would be to completely omit the check for QueryRootExpression but I'm sure it was added for a reason
AsyncEnumerable.cs:21
if (_innerEnumerable is IQueryable<T> queryable)
Any other ideas?
Would be more than happy to submit another PR with whatever you decide on.
The inclusion of the queryable.Expression is Microsoft.EntityFrameworkCore.Query.QueryRootExpression is to satisfy a cast within the FromSql* extension. Invoking the latter on a set where the expression is not a QueryRootExpression causes sad faces.
I think the FromSql* extensions can only be invoked on sets now - in the past I think they were available to IQueryable<T> (could be wrong!) so it may be ok to open up the check. Opening it up to MethodCallExpression may cause problems with read only sets.
It feels like it boils down to not creating a query root expression for anything other than an entity model type. The async enumerable is somewhat decoupled from the db context and I'd like that to continue for reasons, which rules out one possible solution. The other would be to just check that typeof(T) is a non-interface reference type; if it isn't, use the expression from the source enumerable. I'd need to do more testing to know whether that'd be enough.
Another option that seems appealing is to make the setting of the expression to a query root expression when the set is accessed, rather than always wrapping it by default. Larger change but probably more right.
I have applied a fix for this issue to the efcore 5 and 6 versions and it will be included in the next release. It doesn't affect earlier versions. I ended up going with the latter remediation option.
Releases done, let me know of any issues. I've added a release pipeline now so should be a bit quicker releasing changes going forward.
Nice one, thanks! 👍 I'll test around a bit when I'm back at work on Monday