JasperFx / marten

.NET Transactional Document DB and Event Store on PostgreSQL
https://martendb.io
MIT License
2.75k stars 429 forks source link

Referencing nullable types in OData LINQ queries results in an InvalidOperationException being thrown #3198

Closed iress-ljm closed 2 months ago

iress-ljm commented 2 months ago

Attempting to perform a query using Marten Queryable and ASP.NET OData 8 and seeing the following exception when a nullable type is referenced in the OData $filter:

System.InvalidOperationException: variable '$it' of type 'Api.Model.Order' referenced from scope '', but it is not defined
   at System.Linq.Expressions.Compiler.VariableBinder.Reference(ParameterExpression node, VariableStorageKind storage)
   at System.Linq.Expressions.Compiler.VariableBinder.VisitParameter(ParameterExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitMember(MemberExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitBinary(BinaryExpression node)
   at System.Linq.Expressions.Compiler.VariableBinder.VisitUnary(UnaryExpression node)
   at System.Linq.Expressions.ExpressionVisitor.Visit(ReadOnlyCollection`1 nodes)
   at System.Linq.Expressions.Compiler.VariableBinder.VisitLambda[T](Expression`1 node)
   at System.Linq.Expressions.Compiler.LambdaCompiler.Compile(LambdaExpression lambda)
   at System.Linq.Expressions.Expression`1.Compile()
   at FastExpressionCompiler.ExpressionCompiler.CompileSys[TDelegate](Expression`1 lambdaExpr) in /_/src/FastExpressionCompiler/FastExpressionCompiler.cs:line 171
   at FastExpressionCompiler.ExpressionCompiler.CompileFast[R](Expression`1 lambdaExpr, Boolean ifFastFailedReturnNull, CompilerFlags flags) in /_/src/FastExpressionCompiler/FastExpressionCompiler.cs:line 193
   at Marten.Linq.Parsing.LinqInternalExtensions.ReduceToConstant(Expression expression)
   at Marten.Linq.Parsing.SimpleExpression..ctor(IQueryableMemberCollection queryableMembers, Expression expression)
   at Marten.Linq.Parsing.WhereClauseParser.VisitBinary(BinaryExpression node)
   at Marten.Linq.Parsing.WhereClauseParser.Visit(Expression node)
   at Marten.Linq.SqlGeneration.Statement.ParseWhereClause(IReadOnlyList`1 wheres, IMartenSession session, IQueryableMemberCollection collection, IDocumentStorage storage)
   at Marten.Linq.CollectionUsage.BuildTopStatement(IMartenSession session, IQueryableMemberCollection collection, IDocumentStorage storage, QueryStatistics statistics)
   at Marten.Linq.Parsing.LinqQueryParser.BuildStatements()
   at Marten.Linq.Parsing.LinqQueryParser.BuildHandler[TResult]()
   at Marten.Linq.MartenLinqQueryProvider.Execute[TResult](Expression expression)
   at Marten.Linq.MartenLinqQueryable`1.System.Collections.IEnumerable.GetEnumerator()
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeList(JsonWriter writer, IEnumerable values, JsonArrayContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeValue(JsonWriter writer, Object value, JsonContract valueContract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerProperty)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType)
   at Newtonsoft.Json.JsonSerializer.SerializeInternal(JsonWriter jsonWriter, Object value, Type objectType)
   at Newtonsoft.Json.JsonSerializer.Serialize(JsonWriter jsonWriter, Object value)
   at Microsoft.AspNetCore.Mvc.Formatters.NewtonsoftJsonOutputFormatter.WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding selectedEncoding)
   at Microsoft.AspNetCore.Mvc.Formatters.NewtonsoftJsonOutputFormatter.WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding selectedEncoding)
   at Microsoft.AspNetCore.Mvc.Formatters.NewtonsoftJsonOutputFormatter.WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding selectedEncoding)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeResultAsync>g__Logged|22_0(ResourceInvoker invoker, IActionResult result)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|30_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeResultFilters()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Prometheus.HttpMetrics.HttpRequestDurationMiddleware.Invoke(HttpContext context)
   at Prometheus.HttpMetrics.HttpRequestCountMiddleware.Invoke(HttpContext context)
   at Prometheus.HttpMetrics.HttpInProgressMiddleware.Invoke(HttpContext context)
   at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Serilog.AspNetCore.RequestLoggingMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

An failing query would look something like http://api:8080/v1/orders?$filter=Price eq 100 where Price is a nullable decimal. Seems to work fine for non-nullable types.

Sample of the controller:

using Api.Data;
using Api.Model;
using Microsoft.AspNetCore.OData.Routing.Controllers;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.OData.Query;

namespace Buyside.OrdersAndTrades.Api.Controllers;

[Authorize]
[Route("/v1/orders")]
public sealed class OrderController(IOrderRepository orderRepository) : ODataController
{
    [HttpGet]
    [EnableQuery]
    public IQueryable<Order> GetAllOrders()
    {
        return orderRepository.GetAllOrders();
    }
}

Sample of the repository:

using Api.Model;
using Api.Queries;
using Marten;

namespace Buyside.OrdersAndTrades.Api.Data;

public class OrderRepository(IDocumentStore store) : IOrderRepository
{
    private readonly IDocumentStore _store = store;

    public IQueryable<Order> GetAllOrders()
    {
        using var session = CreateSession();

        return session.Query<Order>();
    }

    private IDocumentSession CreateSession() => _store.LightweightSession();
}

Sample of the Order DTO:

public sealed record Order
{
    public Guid Id { get; init; }

    public Guid Account { get; init; }

    public decimal? Price { get; init; }

    public decimal Volume { get; init; }
}
jeremydmiller commented 2 months ago

@iress-ljm Is there any chance you'd be up for building a new web project in a PR that reproduces that issue? I'm not terribly aware of any real testing w/ Marten within oData. A little bit with Hot Chocolate.

iress-ljm commented 2 months ago

@jeremydmiller Happy to do that yes, I'll look at building that out now. Thanks

iress-ljm commented 2 months ago

@jeremydmiller I've raised https://github.com/JasperFx/marten/pull/3201 as a draft PR, it adds a very basic WeatherForecast OData API which uses Marten. It utilises visual studio container tools to launch the API and a postgres container, but if you're unable to run it this way let me know and I can update PR.

Once it's running you should be able to hit the GET endpoint via http://localhost:8080/weatherforecast. There's no data seeded to the database but you should still be able to recreate the issue. Swagger doesn't support OData queries, but if you use an HTTP client like Postman, calling http://localhost:8080/weatherforecast?$filter=Humidity eq 100 will result in an exception being thrown, as Humidity is a nullable decimal. Hope this helps, thank you for taking a look!

jeremydmiller commented 2 months ago

Thanks! I'll switch the usage over to the existing docker compose usage. No Visual Studio usage here:)

And I'll get around to setting up Alba specs on it you might wanna add to your toolbelt.

jeremydmiller commented 2 months ago

@iress-ljm Pulled the PR in today and played with it a bit. As usual, Anything to do with LINQ is time consuming. I see the issue, but I'll have to come back to it after knocking out some client work first.

oData formulates the LINQ Where() clause in a goofy way I didn't anticipate:

(x.Property == value) == True instead of x.Property == value like a human being would write

Just gotta teach the LINQ provider how to handle this one first

iress-ljm commented 2 months ago

@jeremydmiller Thanks for such a quick turnaround on this, much appreciated!