AspNetCoreOData icon indicating copy to clipboard operation
AspNetCoreOData copied to clipboard

Missing Nullable="false" if navigation property

Open ericcanadas opened this issue 3 years ago • 6 comments

Assemblies affected ASP.NET Core OData 8.x

Describe the bug Nullable="false" is missing on the property if a navigation property is present.

Reproduce steps 2 simples class, generating project, see $metadata

Data Model

public class Author
{
    public int AuthorId { get; set; }
    public string AuthorName { get; set; }
}
public class Book
{
    public int BookId { get; set; }
    public string BookTitle { get; set; }
    public int AuthorId { get; set; }

    public virtual Author Author { get; set; }
}

EDM Builder

var edmBuilder = new ODataConventionModelBuilder();
edmBuilder.EntitySet<Author>("Author");
edmBuilder.EntitySet<Book>("Book");

EDM (CSDL) Model

<edmx:Edmx xmlns:edmx="http://docs.oasis-open.org/odata/ns/edmx" Version="4.0">
  <edmx:DataServices>
    <Schema xmlns="http://docs.oasis-open.org/odata/ns/edm">
      <EntityType Name="Author">
        <Key>
          <PropertyRef Name="AuthorId"/>
        </Key>
        <Property Name="AuthorId" Type="Edm.Int32" Nullable="false"/>
        <Property Name="AuthorName" Type="Edm.String"/>
      </EntityType>
      <EntityType Name="Book">
        <Key>
          <PropertyRef Name="BookId"/>
        </Key>
        <Property Name="BookId" Type="Edm.Int32" Nullable="false"/>
        <Property Name="BookTitle" Type="Edm.String"/>
        <Property Name="AuthorId" Type="Edm.Int32"/>
        <NavigationProperty Name="Author" Type="Author">
          <ReferentialConstraint Property="AuthorId" ReferencedProperty="AuthorId"/>
        </NavigationProperty>
      </EntityType>
    </Schema>
    <Schema xmlns="http://docs.oasis-open.org/odata/ns/edm" Namespace="Default">
      <EntityContainer Name="Container">
        <EntitySet Name="Author" EntityType="Author"/>
        <EntitySet Name="Book" EntityType="Book">
          <NavigationPropertyBinding Path="Author" Target="Author"/>
        </EntitySet>
      </EntityContainer>
    </Schema>
  </edmx:DataServices>
</edmx:Edmx>

Expected behavior

<edmx:Edmx xmlns:edmx="http://docs.oasis-open.org/odata/ns/edmx" Version="4.0">
  <edmx:DataServices>
    <Schema xmlns="http://docs.oasis-open.org/odata/ns/edm">
      <EntityType Name="Author">
        <Key>
          <PropertyRef Name="AuthorId"/>
        </Key>
        <Property Name="AuthorId" Type="Edm.Int32" Nullable="false"/>
        <Property Name="AuthorName" Type="Edm.String"/>
      </EntityType>
      <EntityType Name="Book">
        <Key>
          <PropertyRef Name="BookId"/>
        </Key>
        <Property Name="BookId" Type="Edm.Int32" Nullable="false"/>
        <Property Name="BookTitle" Type="Edm.String"/>
        <Property Name="AuthorId" Type="Edm.Int32" Nullable="false"/>
        <NavigationProperty Name="Author" Type="Author" Nullable="false">
          <ReferentialConstraint Property="AuthorId" ReferencedProperty="AuthorId"/>
        </NavigationProperty>
      </EntityType>
    </Schema>
    <Schema xmlns="http://docs.oasis-open.org/odata/ns/edm" Namespace="Default">
      <EntityContainer Name="Container">
        <EntitySet Name="Author" EntityType="Author"/>
        <EntitySet Name="Book" EntityType="Book">
          <NavigationPropertyBinding Path="Author" Target="Author"/>
        </EntitySet>
      </EntityContainer>
    </Schema>
  </edmx:DataServices>
</edmx:Edmx>

<Property Name="AuthorId" Type="Edm.Int32" Nullable="false"/> <NavigationProperty Name="Author" Type="Author" Nullable="false"> <ReferentialConstraint Property="AuthorId" ReferencedProperty="AuthorId"/> </NavigationProperty>

If I comment the navigation property, I get the good return : <Property Name="AuthorId" Type="Edm.Int32" Nullable="false"/>

ericcanadas avatar May 09 '22 13:05 ericcanadas

@ericcanadas That's related to the foreign key configuration.

By design, for any navigation property has a corresponding 'Key' property defined in the same entity type, OData will build the reference constraint between them.

In your scenario, Author is the navigation property, AuthorId is the corresponding 'key' property.

public int AuthorId { get; set; }
public virtual Author Author { get; set; }

Then, a reference constraint are built as:

<ReferentialConstraint Property="AuthorId" ReferencedProperty="AuthorId"/>

In this case, the nullable value is determined by the nullable of navigation property.

You can decorate [Required] on the navigation property or call IsRequired() fluent API to achieve your goal.

Let us know the result. Thanks.

xuzhg avatar May 09 '22 16:05 xuzhg

@xuzhg , since the key type in this case is int and not int?, wouldn't it make sense to infer that the relationship is required by default? I believe that's what would happen in EF as well.

julealgon avatar May 09 '22 19:05 julealgon

@julealgon At the design of the foreign key features, we thought the navigation property is 'significant'. Do you think we wouldn't get a complaint if we changed to use 'key' property?

xuzhg avatar May 09 '22 21:05 xuzhg

@xuzhg My model is generated from database with Scaffold (Entity Framework Tools) I don't want to add [Required] because the files are generated. I can use IsRequired() fluent API (that's what I started to do) but I have a lot of entities and I wondered if it's not a bug finally.

My client library (Odata connected Service) generate a model with this key not mandatory while it is not... In a database, a foreign key can be mandatory or not, but here we do not make a difference between the 2 cases. I'm agree with @julealgon, it make sense to infer this requirement.

Wouldn't it be possible to have an option to choose the behavior?

ericcanadas avatar May 10 '22 08:05 ericcanadas

@julealgon @ericcanadas Here's the existing code: https://github.com/OData/ModelBuilder/blob/main/src/Microsoft.OData.ModelBuilder/Property/NavigationPropertyConfiguration.cs#L232-L244

Changing the code is easy. I just need confirmation. @chrisspre @mikepizzo

xuzhg avatar May 11 '22 23:05 xuzhg

any news ?

ericcanadas avatar May 18 '22 10:05 ericcanadas

This bug is still present.

Besides, I even discovered a more serious case, when there is a foreign key on the primary key of the same table. Example in our case: If in the Book table, we want to store the id of another book like for example PreviousOpusBookId to store the id of the book it follows Well in this case, the primary key of the book table loses its Nullable="false" attribute, which is annoying for a primary key.

@xuzhg do you have an idea how I could do to fix this by overloading a class or with another means? Because it seems that this problem only bothers me...

ericcanadas avatar Mar 23 '23 14:03 ericcanadas