InvalidCastException when filtering on nullable value types

[marcasburnett]

I have a type with nullable DateTime properties, which in some cases I need to add to my FilterCondition. When I do that, I get the error below. It looks like this is because NullableValueTypeDdbConverter inherits from DdbConverter but changes the nullability of the type parameter, which for value types means a totally different type.

Unable to cast object of type 'EfficientDynamoDb.Internal.Converters.NullableValueTypeDdbConverter1[System.DateTime]' to type 'EfficientDynamoDb.Converters.DdbConverter1[System.DateTime]'.
at EfficientDynamoDb.FluentCondition.Core.FilterBase1.GetPropertyConverter[TProperty](DdbExpressionVisitor visitor, Boolean useSize) at EfficientDynamoDb.FluentCondition.Operators.Common.FilterLessThan2.WriteAttributeValues(DdbWriter& writer, DynamoDbContextMetadata metadata, Int32& valuesCount, DdbExpressionVisitor visitor)
at EfficientDynamoDb.FluentCondition.Core.FilterOrWrapper.WriteAttributeValues(DdbWriter& writer, DynamoDbContextMetadata metadata, Int32& valuesCount, DdbExpressionVisitor visitor)
at EfficientDynamoDb.FluentCondition.Core.FilterOrWrapper.WriteAttributeValues(DdbWriter& writer, DynamoDbContextMetadata metadata, Int32& valuesCount, DdbExpressionVisitor visitor)
at EfficientDynamoDb.FluentCondition.Core.FilterAndWrapper.WriteAttributeValues(DdbWriter& writer, DynamoDbContextMetadata metadata, Int32& valuesCount, DdbExpressionVisitor visitor)
at EfficientDynamoDb.Internal.Extensions.Utf8JsonWriterExtensions.WriteConditionExpression(DdbWriter& writer, NoAllocStringBuilder& builder, DdbExpressionVisitor visitor, FilterBase condition, DynamoDbContextMetadata metadata)
at EfficientDynamoDb.Internal.Extensions.Utf8JsonWriterExtensions.WriteConditionExpression(DdbWriter& writer, FilterBase condition, DynamoDbContextMetadata metadata)
at EfficientDynamoDb.Internal.Operations.PutItem.PutItemHighLevelHttpContent.WriteDataAsync(DdbWriter ddbWriter)
at EfficientDynamoDb.Internal.Operations.Shared.DynamoDbHttpContent.SerializeToStreamAsync(Stream stream, TransportContext context)
at EfficientDynamoDb.Internal.Operations.Shared.DynamoDbHttpContent.SerializeToStreamAsync(Stream stream, TransportContext context)
at EfficientDynamoDb.Internal.Operations.Shared.DynamoDbHttpContent.CreatePooledContentReadStreamAsync()
at EfficientDynamoDb.Internal.HttpApi.SendAsync(DynamoDbContextConfig config, HttpContent httpContent, CancellationToken cancellationToken)
at EfficientDynamoDb.DynamoDbContext.PutItemAsync[TEntity](BuilderNode node, CancellationToken cancellationToken)

[anthonybouton]

Also running in this problem
Example Code

     var staleCondition = new FilterOrWrapper(
            Condition<DynamoDbRouteExecutionModel>.On(p => p.LastUpdate).NotExists(),
            Condition<DynamoDbRouteExecutionModel>.On(p => p.LastUpdate).LessThan(reportedAt)
        );
        try
        {
            await _context.UpdateItem<DynamoDbRouteExecutionModel>()
                .WithTableName(_configuration.VehiclesTableName)
                .WithPrimaryKey(primaryKey.PartitionKey, primaryKey.SortKey)
                .On(p => p.Status).Assign(state)
                .On(p => p.PercentageComplete).Assign(percentageComplete)
                .On(p => p.LastUpdate).Assign(new DateTime?(reportedAt))
                .WithCondition(staleCondition)
                .ExecuteAsync(cancellationToken);
        }
        catch (ConditionalCheckFailedException)
        {
            throw new StaleRouteExecutionDataException();
        }

[firenero]

@marcasburnett The issue happens likely because you pass non-nullable value into condition expression that expects a nullable value type. If you pass a nullable value type, the issue goes away.

In example that @anthonybouton provided:

Condition<DynamoDbRouteExecutionModel>.On(p => p.LastUpdate).LessThan(reportedAt)

I assume that p.LastUpdate is DateTime? while reportedAt is a simple DateTime. This is causing an error because converter is not smart enough to handle this case. An easy workaround would be casing reportedAt to nullable DateTime like this:

Condition<DynamoDbRouteExecutionModel>.On(p => p.LastUpdate).LessThan((DateTime?)reportedAt)

It's not great and ideally the code should not compile on type mismatch. There was an old draft PR #193 that does exactly that. However, this is technically a breaking change and I'd need to evaluate if there are potential use-cases that require passing a different type.

[marcasburnett]

@firenero Thank you. I just verified that casting the RHS of the condition to 'DateTime?' works.

In lieu of making a breaking change, maybe a better error message (potentially with a reminder about this quirk) would suffice: something like "Type 'T' used in right-hand side of condition could not be converted to 'T?'. Ensure that nullability of condition parameter matches that of property 'P'."

[firenero]

@marcasburnett I've just merged #273 that makes error message easier to understand. The change is available in 0.9.18-alpha.0.6 if you're willing to try pre-release version.