tomasfabian / ksqlDB.RestApi.Client-DotNet

ksqlDb.RestApi.Client is a C# LINQ-enabled client API for issuing and consuming ksqlDB push and pull queries and executing statements.
MIT License
93 stars 24 forks source link

Exception in ExtractFieldValue When Using Generics (Assumes FieldInfo Type?) #48

Closed tmose1106 closed 9 months ago

tmose1106 commented 9 months ago

Describe the bug

I have created the following interface:

public interface IIdentifiable
{
    public Guid Id { get; }
}

I have written the following generic function with acts on objects implementing IIdentifiable:

public static async Task<Guid> GetUniqueId<T>(IMyKsqlDbContext dbContext, string tableName) where T : IIdentifiable
{
    while(true)
    {
        Guid uniqueId = Guid.NewGuid();

        T? item = await dbContext.CreatePullQuery<T>(tableName).Where(item => item.Id == uniqueId).FirstOrDefaultAsync();

        if (item == null)
        {
            return uniqueId;
        }
    }
}

When doing so, I find a runtime exception:

System.InvalidCastException: Unable to cast object of type 'System.Reflection.RuntimePropertyInfo' to type 'System.Reflection.FieldInfo'.
   at ksqlDB.RestApi.Client.KSql.Query.Visitors.KSqlVisitor.ExtractFieldValue(MemberExpression memberExpression)
   at ksqlDB.RestApi.Client.KSql.Query.Visitors.KSqlVisitor.VisitMember(MemberExpression memberExpression)
   at ksqlDB.RestApi.Client.KSql.Query.Visitors.BinaryVisitor.VisitMember(MemberExpression memberExpression)
   at ksqlDB.RestApi.Client.KSql.Query.Visitors.KSqlVisitor.Visit(Expression expression)
   at ksqlDB.RestApi.Client.KSql.Query.Visitors.BinaryVisitor.Visit(Expression expression)
   at ksqlDB.RestApi.Client.KSql.Query.Visitors.BinaryVisitor.VisitBinary(BinaryExpression binaryExpression)
   at ksqlDB.RestApi.Client.KSql.Query.Visitors.BinaryVisitor.Visit(Expression expression)
   at ksqlDB.RestApi.Client.KSql.Query.Visitors.KSqlVisitor.VisitBinary(BinaryExpression binaryExpression)
   at ksqlDB.RestApi.Client.KSql.Query.Visitors.KSqlVisitor.Visit(Expression expression)
   at ksqlDB.RestApi.Client.KSql.Query.KSqlQueryGenerator.BuildKSql(Expression expression, QueryContext queryContext)
   at ksqlDB.RestApi.Client.KSql.Query.PullQueries.KPullSet`1.GetDependencies()
   at ksqlDB.RestApi.Client.KSql.Query.PullQueries.KPullSet`1.FirstOrDefaultAsync(CancellationToken cancellationToken)
   at DataClearinghouse.WebBackend.Utility.Common.GetUniqueId[T](IWebBackendKsqlDbContext dbContext, String tableName)
   at DataClearinghouse.WebBackend.Controllers.OrganizationAccessController.Post(Guid organizationId, Guid ownerOrganizationId, String webhookUrl) in C:\Users\TedMoseley\source\repos\DataClearinghouse\WebBackend\Controllers\OrganizationAccessController.cs:line 110
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskOfIActionResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   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__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.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.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

To Reproduce I'm assuming this affects all attempts at creating a pull query within a generic function.

Expected behavior The issue seems to be that ExtractFieldValue assumes the member is a FieldInfo type. I would like this to work with both field types and property types using a check of some sort.

Screenshots N/A

Environment (please complete the following information):

Additional context N/A

tomasfabian commented 9 months ago

Hello @tmose1106, I created a PR #49 that fixes this issue. Could you review it, please?

Thank you in advance.

tmose1106 commented 9 months ago

I've looked over the code and I think it looks great. I also read over and ran the tests which seem adequate for this case.

Thank you for your work 👌

tomasfabian commented 9 months ago

Thank you for the review! I merged the changes into the main branch and published a new Nuget package:

dotnet add package ksqlDb.RestApi.Client --version 3.2.2

Enjoy it!

tmose1106 commented 9 months ago

Just downloaded the new version and it has resolved my issue. I think this is safe to close now. I appreciate it.