efcore / EFCore.NamingConventions

Entity Framework Core plugin to apply naming conventions to table and column names (e.g. snake_case)
Apache License 2.0
716 stars 73 forks source link

Naming convention not applied to tables in ASP.NET Identity #2

Closed kanonlemon closed 4 years ago

kanonlemon commented 4 years ago

.net3.0 core in my code

options.UseNpgsql(
    Configuration.GetConnectionString("DefaultConnection")
)
.UseSnakeCaseNamingConventions()

the result of Migrations shows:

all columns have changed to snaked mode but table names keep the CamelCase

roji commented 4 years ago

Apologies, I was accidentally not subscribed to notifications on this repo.

I cannot reproduce this issue, can you please submit precise steps which reproduce the problem? Here's what I did:

Starting program:

class Program
{
    static async Task Main(string[] args)
    {
        using var ctx = new BlogContext();
        ctx.Database.EnsureDeleted();
        ctx.Database.EnsureCreated();
    }
}

public class BlogContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseNpgsql("...");
    }
}

public class Blog
{
    public int Id { get; set; }
    public string Name { get; set; }
}

I created a migration, then added .UseSnakeCaseNamingConvention() in OnConfiguring, and created a second migration. At this point, when generating a database with the migrations (dotnet ef database drop -f; dotnet ef database update) my database was created with lower-case table names as expected.

pawmot commented 4 years ago

Hi @roji, I don't know if @kanonlemon encountered exactly the same issue as me, but in my case the Identity Core tables were problematic - columns were snake_case, but table names remained CamelCase. I was able to fix this by configuring the model - using just UseSnakeCaseNamingConventions was not enough. When it comes to my own types table names and columns were OK, but e.g. primary keys were still beginning with "PK" instead of "pk".

jasonwarford commented 4 years ago

I'm experiencing the same thing. In my case I'm using dotnet core 3.1 and trying to generate the migrations for Identity Server 4 PersistedGrantDbContext and ConfigurationDbContext.

I'll try to pair it down to a simple example that illustrates the issue.

Here is the code that is generated for one of the tables for instance:

migrationBuilder.CreateTable(
                name: "ApiResources",
                columns: table => new
                {
                    id = table.Column<int>(nullable: false)
                        .Annotation("Npgsql:ValueGenerationStrategy", NpgsqlValueGenerationStrategy.IdentityByDefaultColumn),
                    enabled = table.Column<bool>(nullable: false),
                    name = table.Column<string>(maxLength: 200, nullable: false),
                    display_name = table.Column<string>(maxLength: 200, nullable: true),
                    description = table.Column<string>(maxLength: 1000, nullable: true),
                    created = table.Column<DateTime>(nullable: false),
                    updated = table.Column<DateTime>(nullable: true),
                    last_accessed = table.Column<DateTime>(nullable: true),
                    non_editable = table.Column<bool>(nullable: false)
                },
roji commented 4 years ago

I'll try to pair it down to a simple example that illustrates the issue.

That would be great.

roji commented 4 years ago

tl;dr ~for now~, this plugin won't affect ASP.NET Identity tables - you'll have to apply naming manually (e.g. see this SO).

The problem is specific to the ASP.NET Identity model. Because the model uses generic types (TUser, TRole...), it explicitly configures table names in OnModelCreating. Anything that happens in OnModelCreating takes precedence over conventions, which is what this plugin sets up to apply the naming. In other words, the mechanism of specifying table names in OnModelConfiguring was originally meant to allow users to override conventions, but in this case is being used by ASP.NET Identity in another way.

@ajcvickers @AndriySvyryd any ideas here? Identity could have its own convention to set the names, but then we'd have two competing conventions with no clear ordering...

ajcvickers commented 4 years ago

@roji The ASP.NET Core Identity model is heavily configured--very few conventions are used in its configuration. It's seems pretty complicated to try to go back to using some conventions after this--it's not something I think you can do now. Even if we did, it would be work and a breaking change for Identity to opt into it, which I don't think will happen.

roji commented 4 years ago

That's what I thought. I'll close this for now...

roji commented 4 years ago

Note to anyone hitting this: although the plugin can't apply its naming conventions, you can do this yourself by dropping the following into your OnModelCreating:

modelBuilder.Entity<IdentityUser>().ToTable("asp_net_users");
modelBuilder.Entity<IdentityUserToken<string>>().ToTable("asp_net_user_tokens");
modelBuilder.Entity<IdentityUserLogin<string>>().ToTable("asp_net_user_logins");
modelBuilder.Entity<IdentityUserClaim<string>>().ToTable("asp_net_user_claims");
modelBuilder.Entity<IdentityRole>().ToTable("asp_net_roles");
modelBuilder.Entity<IdentityUserRole<string>>().ToTable("asp_net_user_roles");
modelBuilder.Entity<IdentityRoleClaim<string>>().ToTable("asp_net_role_claims");

(don't forget to change string if you're using another key type)

10GeekJames commented 4 years ago

to make the above compile I needed to add .HasNoKey() to each line. This did help with those tables, thank you. However; I seem to have a different situation where NO tables are getting the lower-case treatment. I do have the project broken across several projects with data, infrastructure, front-end. The front end is where something like dotnet ef database migration would be called. When I implement this code I get lowercase "properties" across the board and zero lowercase table names, any guesses?

roji commented 4 years ago

You should definitely not be adding HasNoKey() to the above - are you sure you're using string as your key? If not, you need to change the generic type parameter above. Otherwise please post your identity DbContext.

Re the 2nd problem - are you saying you're seeing a non-Identity table not being affected by this plugin? If so, can you please open a new issue with a minimal code sample?

igorgomeslima commented 3 years ago

Note to anyone hitting this: although the plugin can't apply its naming conventions, you can do this yourself by dropping the following into your OnModelCreating:

modelBuilder.Entity<IdentityUser>().ToTable("asp_net_users");
modelBuilder.Entity<IdentityUserToken<string>>().ToTable("asp_net_user_tokens");
modelBuilder.Entity<IdentityUserLogin<string>>().ToTable("asp_net_user_logins");
modelBuilder.Entity<IdentityUserClaim<string>>().ToTable("asp_net_user_claims");
modelBuilder.Entity<IdentityRole>().ToTable("asp_net_roles");
modelBuilder.Entity<IdentityUserRole<string>>().ToTable("asp_net_user_roles");
modelBuilder.Entity<IdentityRoleClaim<string>>().ToTable("asp_net_role_claims");

(don't forget to change string if you're using another key type)

Unfortunately, this not work anymore.

I trying this:

Startup.cs

public void ConfigureServices(IServiceCollection services)
{
    services.AddControllers();

    services.AddDbContext<ApplicationDbContext>();

    services.AddIdentity<IdentityUser, IdentityRole>()
                .AddEntityFrameworkStores<ApplicationDbContext>();
}

ApplicationDbContext.cs

public class ApplicationDbContext : IdentityDbContext
{
    public ApplicationDbContext(DbContextOptions dbContextOptions) : base(dbContextOptions)
    {

    }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        if (!optionsBuilder.IsConfigured)
        {
            optionsBuilder.UseNpgsql("Host=host.docker.internal;Database=identity;Username=postgres;Password=postgres")
                .UseSnakeCaseNamingConvention();
        }
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);
    }
}

csproj With:

<PackageReference Include="EFCore.NamingConventions" Version="5.0.0-rc1" />
<PackageReference Include="Microsoft.AspNetCore.Identity.EntityFrameworkCore" Version="5.0.0" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="5.0.0">
<PackageReference Include="Microsoft.Extensions.Identity.Core" Version="5.0.0" />
<PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="5.0.0" />
<PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL.Design" Version="1.1.0" />
roji commented 3 years ago

@igorgomeslima when you do ToTable, you're explicitly setting table names - EFCore.NamingConventions has nothing to do with anything at that point... Can you please double-check that there's actually a behavioral difference between 5.0.0 and a previous version?

igorgomeslima commented 3 years ago

@igorgomeslima when you do ToTable, you're explicitly setting table names - EFCore.NamingConventions has nothing to do with anything at that point... Can you please double-check that there's actually a behavioral difference between 5.0.0 and a previous version?

@roji With previous versions 5.0.0-preview8 & 5.0.0-preview5 i got the same result. With versions below those, I got the error mentioned here.

At this point: "...(don't forget to change string if you're using another key type)..." if we change the type, for example, modelBuilder.Entity<IdentityRoleClaim<**int**>>().ToTable("asp_net_role_claims"); the migration duplicate table, like this:

Captura de tela 2020-11-17 164132

roji commented 3 years ago

My point is that whatever is happening with ToTable is unrelated to this plugin - you should be able to remove EFCore.NamingConventions and see exactly the same behavior as you see with it.

igorgomeslima commented 3 years ago

My point is that whatever is happening with ToTable is unrelated to this plugin - you should be able to remove EFCore.NamingConventions and see exactly the same behavior as you see with it.

Yes. If remove the plugin call, the result is the same. So I mentioned, this configuration not work.

roji commented 3 years ago

Try posting that on the ASP.NET repo - this repo is for EFCore.NamingConventions.

adiletelf commented 1 year ago

Would be nice to specify which tables need snake_case naming, for example I may want to use default naming for AspNetCore.Identity tables but snake_case for my own. The reason for it is the strange bug:

1) EFCore.NamingConventions renames columns in AspNetCore.Identity tables like Id -> id (as expected). 2) I try to use UserManager<> class which uses the table AspNetUsers to find the user with corresponding id. 3) For some reason instead of using renamed column id (lowercase) it tries to search for Id (uppercase) and I get Exception: "Column 'AspNetUsers.Id' does not exist".

For now I deleted EFCore.NamingConventions and manually change column names.

Update: solved the issue using dynamic model configuration:

public class SnakeDbContext : IdentityDbContext<ApplicationUser, ApplicationRole, int>
{
    public DbSet<Reward> Rewards => Set<Reward>();
    public DbSet<Education> Educations => Set<Education>();
    public DbSet<Citizenship> Citizenships => Set<Citizenship>();
    public DbSet<Position> Positions => Set<Position>();
    public DbSet<RewardApplication> RewardApplications => Set<RewardApplication>();
    public DbSet<Entity> Entities => Set<Entity>();
    public DbSet<Person> People => Set<Person>();
    public DbSet<Citizen> Citizens => Set<Citizen>();
    public DbSet<Mother> Mothers => Set<Mother>();
    public DbSet<Foreign> Foreigners => Set<Foreign>();

    protected override void OnModelCreating(ModelBuilder builder)
    {
        base.OnModelCreating(builder);

        var entities = builder.Model.GetEntityTypes();
        foreach (var entity in entities)
        {
            // Cannot invoke .ToTable for abstract classes when using TPC
            // BaseEntity is the base class for user defined classes, does not change AspNetCore.Identity tables
            if (!typeof(BaseEntity).IsAssignableFrom(entity.ClrType) || entity.IsAbstract())
            {
                continue;
            }

            // returns name of DbSet<TEntity> instead of TEntity
            var currentTableName = builder.Entity(entity.Name).Metadata.GetTableName();
            if (string.IsNullOrEmpty(currentTableName))
            {
                continue;
            }
            builder.Entity(entity.Name).ToTable(GetSnakeName(currentTableName));

            foreach (var property in entity.GetProperties())
            {
                builder.Entity(entity.Name).Property(property.Name).HasColumnName(GetSnakeName(property.Name));
            }
        }
    }

    private string GetSnakeName(string name)
    {
        return string.Concat(
            name.Select((x, i) => i > 0 && char.IsUpper(x)
                ? $"_{x}"
                : x.ToString())).ToLower();
    }
}