TrackableEntities / EntityFrameworkCore.Scaffolding.Handlebars

Scaffold EF Core models using Handlebars templates.
MIT License
208 stars 53 forks source link

[Bug] Enabling Nullable Reference Types does not make navigation properties nullable #151

Closed Leosori closed 3 years ago

Leosori commented 3 years ago

Describe the bug If EnableNullableReferenceTypes is set to true, non-collection nav properties that belong to a nullable property are not marked as nullable. This causes Entity Framework to use INNER JOINs instead of OUTER JOINs when generating the SQL query.

Example of the currently generated entity:

class Person
{
    public int Id {get; set;}
    public string Name {get; set;}
    public int FirstAddressId {get; set;}
    public int? SecondAddressId {get; set;}

    // nav property
    public virtual Address FirstAddress { get; set; } = default!;
    public virtual Address SecondAddress { get; set; } = default!;
}

Expected behavior

class Person
{
    public int Id {get; set;}
    public string Name {get; set;}
    public int FirstAddressId {get; set;}
    public int? SecondAddressId {get; set;}

    // nav property
    public virtual Address FirstAddress { get; set; } = default!;
    public virtual Address? SecondAddress { get; set; }
}

Additional context Property template looks currently like this and uses property-isnullable

public {{property-type}} {{property-name}} { get; set; }{{#if nullable-reference-types }}{{#unless property-isnullable}} = default!;{{/unless}}{{/if}}

Nav properties have this template:

{{#if nav-property-collection}}
        public virtual ICollection<{{nav-property-type}}> {{nav-property-name}} { get; set; }{{#if nullable-reference-types}} = default!;{{/if}}
{{else}}
        public virtual {{nav-property-type}} {{nav-property-name}} { get; set; }{{#if nullable-reference-types}} = default!;{{/if}}
{{/if}}

In order to get it right, the nav property template needs access to property-isnullable

{{#if nav-property-collection}}
        public virtual ICollection<{{nav-property-type}}> {{nav-property-name}} { get; set; }{{#if nullable-reference-types}} = default!;{{/if}}
{{else}}
        public virtual {{nav-property-type}}{{#if nullable-reference-types}}{{#if property-isnullable}}?{{/if}}{{/if}} {{nav-property-name}} { get; set; }{{#if nullable-reference-types}}{{#unless property-isnullable}} = default!;{{/unless}}{{/if}}
{{/if}}

AFAIK should the collection variant stay non-nullable because this is covered by an empty collection.

tonysneed commented 3 years ago

I will see if I can replicate this behavior.

tonysneed commented 3 years ago

In order to surface a nav-property-isnullable template item, we need to determine whether a navigation property is optional, where the foreign key is nullable. I looked at INavigation, but I could not find an extension method that would reveal this.

@bricelam, Can you point us in the right direction? [Nevermind, @kevin-a-naude submitted a PR with the fix.]