Should navigation property be nullable or not

Question

Look at this entity:

class EntityA
{
   public int Id { get;set; }
   public string Name { get;set; }
   public int? ClientId { get; set; }

   // Navigation properties:
   public ClientEntity? Client { get; set; }
}

As you can see, this entity contains an optional property: ClientId. This means the client is optional. In this case, the ClientId field will contain NULL in the sql server database.

I am working with navigation properties for foreign keys: This is the "Client" property. When ClientId is null, Client should be null too.

This is why i have declared: "ClientEntity?" type for the Client property.

But i see people who declare "ClientEntity" (not nullable) in same circumstances. But i do not understand how they can manipulate null clients in this case...

Any idea ?

Thanks

Answer 1

This excellent question is missing an excellent answer, so to anyone who landed here seeking guidance...

Short answer

When NRT enabled, the navigation property should be nullable when

1. The DB relationship is not mandatory.
2. You are not sure if the navigation will be loaded or not.

Otherwise, the navigation properties could be defined as non-nullable, marked as required, or the NRT warning issued by compiler turned off by = null!.

Long answer

These relationships and

Entity definition

Let's image an entity Customer that has some elementary properties FullName and Age. The FullName must be provided, the Age is optional.

Each customer must have an address and could have a discount code, both non-trivial entities stored in the same DB, but different tables

In addition, the Customer entity has 1:N relation in DB for Orders.

So... We have basically all possible options that we can encounter with.

public class Customer
{
   // In DB, set as mandatory, but EF take care about this.
   // By default, initiated to 0
   public int Id {get; set;}
   
   // In DB, set as mandatory, we could take care about this,
   // or set it up in DB as auto value.
   // By default initiated on Guid.Empty.
   public Guid UniqueId {get; set;}
   
   // In DB, set as mandatory.
   public string FullName {get; set;}
   
   // In Db, set as optional (can have null).
   public int Age {get; set;}
   
   // In Db, set as mandatory.
   public int AddressId {get; set;}
   
   public Address Address {get; set;}
      
   // In Db, set as optional relationship (it might not exists).
   public int DiscountCodeId {get; set;}
   
   public DiscountCode DiscountCode {get; set;}

   public ICollection<Order> Orders {get; set;}
}

Possible approaches

First of all, you have to distinguish between basic values that can be represented by some elementary type (int, double, datetime, string, etc.) and any non-elementary object that represent a navigation property.

Second of all, you have to set the standard that you want to follow, either the defensive one, or the optimistic one.

The showcase for both cases for .NET 8 and C# 12 follows.

Defensive approach

public class Customer
{
   // In DB, set as mandatory.
   public int Id {get; set;}
   
   // In DB, set as mandatory.
   public Guid UniqueId {get; set;}
   
   // In DB, set as mandatory.
   public required string FullName {get; set;}
   
   // In Db, set as optional (can have null).
   public int? Age {get; set;}
   
   // In Db, set as mandatory.
   public required int AddressId {get; set;}
   
   public Address? Address {get; set;}
      
   // In Db, set as optional relationship.
   public int? DiscountCodeId {get; set;}
   
   public DiscountCode? DiscountCode {get; set;}

   public ICollection<Order> Orders {get; set;} = [];
}

FullName is marked as required because it is required by the DB.

AddressId could be marked required, but it depends. It will be required every single time you want to create a Customer instance, which could be tedious when writing tests. Generally, I would set it as required and use the builder pattern to create Customer in tests.”

Address is set as nullable because Address has to be loaded in order to be not null. In the defensive approach, we want to force null checks upon use because it doesn’t have to be loaded.

Age is set as nullable because it is not required and might be null.

DiscountCodeId is set as nullable because it is not required and might be null.

DiscountCode is set as nullable because whole relationship is nullable, so when DiscountCodeId is null, DiscountCode must be null accordingly.

Orders are not marked as null because if there are no orders for a given customer, that should be indicated only by an empty collection. So, we just initialize the property with an empty list. EF Core will override this with a collection of orders if there are any orders.

This setup will generate no nullable reference type warning.

If you do load navigation properties by Include(...) method(s), all of these navigation properties should be marked as required automatically.

Any entity that does not have all navigation property set will become null after calling Include(...) that touches a navigation property that is not set.

public class RideRequest
{
  public int Id {get; set;}
  
  public Location Pickup {get; set;}

  public Location Dropff {get; set;}
}

public class SomeRepository
{
  // Some loading method
  public IQueryable<RideRequest> IncludeNavigationProperty(IQueryable<RideRequest> query) =>
    query
    .Include(x => x.Pickup)
    .Include(x => x.Dropoff);
}

// Some unit test code
var rideRequestId = 1;
var options = GetDbContextOptions();
await using (var context = GetDbContext(options))
{
    var rideRequestSeed = new RideRequest { Id = rideRequestId, Pickup = new() };
    _ = await context.RideRequests.AddAsync(rideRequestSeed, CancellationToken.None);
    _ = await context.SaveChangesAsync(CancellationToken.None);
}

var repository = GetRepository(GetDbContext(options));

// This method will call IncludeNavigationProperty() on behind
var rideRequest = await repository.GetRideRequestById(rideRequestId, CancellationToken.None);

// Now, rideRequest is null despite the fact that the entity do exists in DB,
// because of missing Dropoff property.

// In order to fix this, we must provide Dropoff
var rideRequestSeed = new RideRequest { Id = rideRequestId, Pickup = new(), Dropoff = new() };

// Hence, marking both properties as required makes sense.

Optimistic approach

public class Customer
{
   // In DB, set as mandatory.
   public int Id {get; set;}
   
   // In DB, set as mandatory.
   public Guid UniqueId {get; set;}
   
   // In DB, set as mandatory.
   public required string FullName {get; set;}
   
   // In Db, set as optional (can have null).
   public int? Age {get; set;}
   
   // In Db, set as mandatory.
   public int AddressId {get; set;}
   
   public Address Address {get; set;} = null!
      
   // In Db, set as optional relationship.
   public int? DiscountCodeId {get; set;}
   
   public DiscountCode? DiscountCode {get; set;}

   public ICollection<Order> Orders {get; set;} = [];
}

The only difference is in the navigation properties. All other cases remain the same. In this approach, we expect that Address is automatically loaded.

AddressId is not marked as required, and the developer is responsible for setting the value correctly when a new object of this class is created.

Address is set as non-nullable, which triggers a compiler warning about a possible null value after instance initialization. This is correct. The warning is suppressed by the null forgiveness operator = null!.

Test driven approach

If you don't use Include(...) methods, you can write a test to verify that navigation properties are properly loaded and than you can safely use = null! in your domain classes to force out compiler warnings on navigation properties.

The test(s) will creates a safe way how to access your navigation properties directly.

For such a test, we can use InMemory provider, or TestContainers.

A real-life example

public class RideRequestTests : RepositoryTest<RidesRepository>
{
    public class LocationTests
    {
        [Fact(DisplayName = "PickupLocation & DropOffLocation should not be null")]
        public async Task LocationsMustBeLoadedTest()
        {
            var rideRequestId = 1;
            var options = GetDbContextOptions();
            await using (var context = GetDbContext(options))
            {
                var rideRequestSeed = new RideRequest { Id = rideRequestId, PickupLocation = new(), DropoffLocation = new() };
                _ = await context.RideRequests.AddAsync(rideRequestSeed, CancellationToken.None);
                _ = await context.SaveChangesAsync(CancellationToken.None);
            }

            var repository = GetRepository(GetDbContext(options));

            var rideRequest = await repository.GetRideRequestById(rideRequestId, CancellationToken.None);

            _ = rideRequest
                .Should()
                .NotBeNull()
                .And
                .Match<RideRequest>(rr => rr.PickupLocation != null! && rr.DropoffLocation != null!, because: "both properties are required because of Include statement");
            _ = rideRequest!.PickupLocation.Id.Should().NotBeEmpty(because: "ORM should assign ID to the entity");
            _ = rideRequest.DropoffLocation.Id.Should().NotBeEmpty(because: "ORM should assign ID to the entity");
        }
    }
}

public abstract class RepositoryTest<TRepository>
    where TRepository : class, ICommonRepository
{
    protected static TRepository GetRepository(MyAppDbContext? dbContext = null) =>
        Activator.CreateInstance(typeof(TRepository), dbContext ?? GetDbContext()) as TRepository
        ?? throw new InvalidCastException($"The provided repository of type {typeof(TRepository).FullName} cannot be casted to itself, most likely due to missing constructor that takes {typeof(MyAppDbContext).FullName}");

    protected static MyAppDbContext GetDbContext(DbContextOptions<MyAppDbContext>? options = null)
    {
        var context = new MyAppDbContext(options ?? GetDbContextOptions());
        return context;
    }

    protected static DbContextOptions<CityaDbContext> GetDbContextOptions() =>
        new DbContextOptionsBuilder<MyAppDbContext>().UseInMemoryDatabase($"Test{Guid.NewGuid()}").Options;
}