Fix nullability for Fluent API #24457
Conversation
| @@ -79,7 +79,7 @@ public class CollectionNavigationBuilder<TEntity, TRelatedEntity> : CollectionNa | |||
| /// </param> | |||
| /// <returns> An object to further configure the relationship. </returns> | |||
| public virtual ReferenceCollectionBuilder<TEntity, TRelatedEntity> WithOne( | |||
| Expression<Func<TRelatedEntity, TEntity>>? navigationExpression) | |||
| Expression<Func<TRelatedEntity, TEntity?>>? navigationExpression) | |||
There was a problem hiding this comment.
Am curious, what's the need for this (here and everywhere below)?
There was a problem hiding this comment.
To allow configuring nullable navigations (since TEntity is marked as non-nullable)
There was a problem hiding this comment.
Ah of course.
One interesting question this raises... rather than changing the lambda's return type, we could change the generic constraint for TRelatedEntity to be class? instead of class. This would flow the nullability of the property to the returned ReferenceCollectionBuilder - which may be good, but also bad. For example:
modelBuilder.Entity<Blog>()
.HasOne(b => b.Details)
.WithOne(d => d.Blog);If Blog.Details is nullable and HasOne above flows the nullability to ReferenceNavigationBuilder, then you get a warning in WithOne (this can be fixed by having WithOne accept a lambda with a nullable first type parameter as well).
I guess the question is... is there value in flowing the nullability of d.Details onwards? It feels slightly more correct, but I'm not sure if we actually need it or not.
Another unrelated idea - I think we discourage nullable collection navigations. Not sure to what extent this is supported/unsupported, but if we really want to discourage it we can disallow it via annotation.
There was a problem hiding this comment.
Flowing the nullability means one would need to use a bang WithOne(d => d!.Blog);, also keeping the non-nullable constraint is more correct as it's applied to the entity type, not the navigation type.
We handle nullable collections fine, what we don't allow are nulls in the collection, that's what the signature still restricts
No description provided.