No way to create a resource derived from Identifiable<string> without sending Id from client side

[minhhieugma]

DESCRIPTION

Cannot make a client POST request to create a resource derived from Identifiable<string>

• Controller validates the model saying The Id field is required.

For many reasons, we might want to assign the Id from the server-side. In the 4.x version, I assign an Id at OnWritingAsync. However, with the 5.x version, we validate the ModelState before reaching the OnWritingAsync function

STEPS TO REPRODUCE

public sealed class RgbColor : Identifiable<string>
{
    [Attr]
    public string DisplayName { get; set; } = null!;

    [HasOne]
    public WorkItemGroup? Group { get; set; }
}

options.AllowClientGeneratedIds = false;
options.ValidateModelState = true;

POST request with the request body

{
   "data":{
      "type":"rgbColor",
      "attributes":{
         "DisplayName":"Blue"
      }
   }
}

EXPECTED BEHAVIOR

• Allow assigning Id from server-side
• Don't throw the Modal Validation with Id

ACTUAL BEHAVIOR

• The controller always validates the model and throws The Id field is required. exception

VERSIONS USED

• JsonApiDotNetCore version: 5.0.1
• ASP.NET Core version: 6.0
• Entity Framework Core version: 6.0
• Database provider: InMemory

[bkoelman]

Hi @minhhieugma, thanks for the provided repro.

ModelState validation hasn't changed in v5. The error you're observing is caused by enabling nullable reference types. That changes how ASP.NET validates models: it assumes reference types are required by default.

To get the same behavior as in v4.x, there are several options:

• Disable nullable reference types in your project.
• Set MvcOptions.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes:

  var mvcBuilder = services.AddMvcCore(options =>
      options.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes = true);
  services.AddJsonApi<AppDbContext>(mvcBuilder: mvcBuilder);

• Change TId from string to string? and override the Id property:

  public sealed class RgbColor : Identifiable<string?>
  {
      public override string? Id { get; set; }
  
      [Attr]
      public string DisplayName { get; set; } = null!;
  }

  I didn't investigate why overriding the property is needed, but found it doesn't work otherwise.

[bkoelman]

Hi @minhhieugma, does this solve your problem? Do you need this issue to remain open?

[minhhieugma]

I think we can close the issue for now. Even though, with the same solution of JADNC 4.2 and .NET 3.1 and EF 3.1

• Update to 4.2 + .NET 6 + EF 5 -> work normal, no need to do anything
• Update to 5.x + .NET 6 + EF 6 -> need to change string to string?

Thank you @bkoelman BTW

[bkoelman]

Thanks for the extra details. One difference between v4 and v5 is that v5 is compiled with nullable reference types enabled. This changes how ASP.NET ModelState interprets the required-ness of Identifiable.Id.

Yesterday I started digging into this some more, but unfortunately my time is limited. I'll see if I can continue my investigation during the weekend. Depending on my observations, I'll reopen this issue.

[bkoelman]

I've taken a deep dive and found the root cause.

Using a minimal repro project, I was unable to get the same validation error. So I started comparing the compiler-generated nullability attributes in the DLL using ILDASM and found interesting differences. The JsonApiDotNetCore.Annotations assembly contained [NullableContextAttribute] usages that weren't in the DLL from my simplified repro project:

public abstract class Identifiable<TId>
{
    public virtual TId Id { get; set; } = default!;
}

public class Customer : Identifiable<string>
{
    public string Name { get; set; } = null!;

    [Range(1, 150)]
    public int Age { get; set; }
}

As described here, this is because the StringId and LocalId properties are missing in my simplified version of Identifiable:

The C# compiler doesn't just use NullableAttribute to mark which values are nullable, it also saves up on the assembly size by compacting several NullableAttributes into a single NullableContextAttribute.

The exact algorithm is described in the nullable-metadata.md file but it's along the lines of "if a class has more nullable members than non-nullable members, annotate only the non-nullable members with NullableAttribute and mark the class itself as nullable using NullableContextAttribute.

There's a bug in ASP.NET 6: it duplicates compiler logic to determine nullability, but takes some shortcuts and gets it wrong. This was recently fixed by replacing the custom logic with relying on the NullabilityInfoContext reflection API in the runtime. I've created a bug report here.

But it doesn't stop there. Turns out that NullabilityInfoContext in .NET 6 does not work correctly for our case (see the end of my bug report). There are several recent fixes to that, see https://github.com/dotnet/runtime/issues?q=is%3Aclosed+NullabilityInfoContext+label%3Aarea-System.Reflection.

So I tried with .NET 7 preview 4, where everything works as expected. The solution I recommend to you: use option 3 from my earlier comment. When updating to .NET 7, you can remove the Id property override.