src/Identity/Extensions.Stores/src/IdentityRole.cs Name Should be required and not nullable

[luizfbicalho]

Summary

I didn't understand why IdentityRole.Name, IdentityRole.NormalizedName , IdentityUser.UserName and IdentityUser.NormalizedUserName are nullable, this are required properties and should be required

Motivation and goals

The major problem that I see is that i every place that I use the IdentityRole and IdentityUser I'll need to test if the name is null, and doesn't seem to be a reason for that.

The issue is in the #40007 change

[HaoK]

We have a constructor that leaves UserName null for example here https://github.com/dotnet/aspnetcore/blob/main/src/Identity/Extensions.Stores/src/IdentityUser.cs#L19

[luizfbicalho]

Couldn't this constructor leave it as string.empty?

this will make every part of the code to check this properties if they are null

[davidkarlsson]

I don't understand why you made these properties nullable either. These properties should not be nullable for us and making these changes at this late stage seems rather reckless in my opinion considering they are used for database entities.

Are there even any use cases where it makes sense that these properties are nullable? What good is a role without a name here for instance?

Now we have a mismatch between the nullability of the columns in our users and roles tables and our ef core entities and it's incorrect for us to change the nullability of the columns.

[KaisoBits]

I agree that it seems like an unnecessary change especially for a property such as the username.

I can't think of much use cases where you don't have a username for your user so to me it seems like this way you're forcing 95% of projects to put null forgiving operators everywhere where this property is used because it'll never be null.

I guess the workaround would be to do something like this to silence all warnings

public class MyUserClass : IdentityUser
{
  [AllowNull]
  public override string UserName { get; set; }
}

or this to be safe

[AllowNull]
public override string UserName 
{ 
    get => base.UserName ?? string.Empty;
    set => base.UserName = value ?? string.Empty; 
}

AllowNull is necessary to silence the warning about the nullability of the overriden property not matching the base class' one although it has the effect of allowing null assigments to this property.

So maybe for this case simply suppressing this warning in the MyUserClass is a better solution.

#pragma warning disable CS8765 // Nullability of type of parameter doesn't match overridden member (possibly because of nullability attributes).
set => base.UserName = value ?? string.Empty;
#pragma warning restore CS8765

[luizfbicalho]

I still don't get it, I mean, is there a scenario where username should be null?