Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverse Engineering: Enable pluralization/singularization when scaffolding a model from a database #3060

Closed
abelepereira opened this issue Sep 8, 2015 · 25 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@abelepereira
Copy link

Database scaffolding has no pluralization service and the entity models are generated mapping the exact name of database objects. My suggestion is to enable this pluralization through a DI service that could be overridden at the ef CLI like this:

dnx ef dbcontext scaffold "Server=myserver;Database=Foo;user id=me;password=pwd" EntityFramework.SqlServer --use-pluralization "MyAssembly.PlurarizationService, MyAssembly"

And on Razor templates this could be exposed as:

...
@:namespace @Model.Namespace
@:{
@:    public class  @PluralizationService.Singularize(Model.EntityType.Name)
@:    {
...

I think the default service could be the one shipped with EF6 (System.Data.Entity.Design.PluralizationServices.PluralizationService)

@anpete
Copy link
Contributor

anpete commented Sep 8, 2015

We could consider this inflector (which I wrote a long time ago) - https://github.com/srkirkland/Inflector

It is very lightweight (single class, ~200 loc). MIT license.

@rowanmiller
Copy link
Contributor

EF Team Triage: Pluralization caused a lot of issues in EF6, where even a simple bug fix can break existing models. For that reason we are not going to have pluralization on by default in EF7. That said, we are going to have a simple way to get the model before it is passed to code generators so that you can run any logic over the model to manipulate it. Pluralization would be exactly the kind of thing that should be simple to do (and a good way to verify our work).

This ability was tracked as part of some larger work, but I opened #3077 to track this piece.

@divega
Copy link
Contributor

divega commented Feb 3, 2016

Reopening this issue to follow up on the discussion we had in triage. We would like to support pluralization for collection navigation properties and singularization of tables names in scaffolding (which is what the original request was about). We need to discuss if this is really part of the potentially more complex extensibility feature described in #3077 or something we have out of the box.

@divega divega reopened this Feb 3, 2016
@gdoron
Copy link

gdoron commented Feb 11, 2016

@divega I think it is a feature almost all EF < 7 users used so it will be a shame not to have it in EF 7.
But it should be an extension method on DbContext whether it should use the pluralization strategy or not.

@divega
Copy link
Contributor

divega commented Feb 11, 2016

@gdoron this issue is only about using pluralization/singularization in scaffolding, i.e. in the process of reverse engineering a database into an EF Core model.

Past experience has demonstrated that using pluralization to automatically create table names based on the entity types is not a good idea: it means any bug fix in the pluralizer becomes a breaking change for the runtime, so we are not planning to do that. One idea we have talked about is to create a convention that uses the name of the DbSet property on the derived DbContext to name the table. Would that work for you?

@gdoron
Copy link

gdoron commented Feb 11, 2016

@divega it most definitely would and I think it's the best solution (even if the pluralizer had no bugs at all) because this is the way I use it in the code, I expect it to be the same in the DB.

Even if for some weird reason someone has a class name completely different than the DbSet propery name, e.g. Post class with DbSet Comments. I would still say that the DbSet name should be that table name and in this case the table name should be Comments.

@rowanmiller
Copy link
Contributor

@gdoron stick this code at the start of OnModelCreating and it will give you what you are after 😄

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    var dbSets = GetType().GetProperties()
        .Where(p => p.PropertyType.Name == "DbSet`1")
        .Select(p => new
        {
            PropertyName = p.Name,
            EntityType = p.PropertyType.GenericTypeArguments.Single()
        })
        .ToArray();

    foreach (var type in modelBuilder.Model.GetEntityTypes())
    {
        var dbset = dbSets.SingleOrDefault(s => s.EntityType == type.ClrType);
        if(dbset != null)
        {
            type.Relational().TableName = dbset.PropertyName;
        }

    }

    // Other configuration code...
}

@gdoron
Copy link

gdoron commented Feb 11, 2016

Thanks @rowanmiller, in case someone else finds it helpful, I wrapped it in an extension method:

public static void UseDbSetNamesAsTableNames(this DbContext dbContext, ModelBuilder modelBuilder)
{

    var dbSets = dbContext.GetType().GetRuntimeProperties()
        .Where(p => p.PropertyType.Name == "DbSet`1")
        .Select(p => new
                            {
                                PropertyName = p.Name,
                                EntityType = p.PropertyType.GenericTypeArguments.Single()
                            })
        .ToArray();

    foreach (var type in modelBuilder.Model.GetEntityTypes())
    {
        var dbset = dbSets.SingleOrDefault(s => s.EntityType == type.ClrType);
        if (dbset != null)
        {
            type.Relational().TableName = dbset.PropertyName;
        }

    }
}

And used it like this:

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

But I get an error says cannot find the object "Posts"

I checked what EF is trying to execute with dnx ef migrations script 20160211201857_second -o c:\temp\rename.txt and pasted it here.
EF is using the new name Posts before it was renamed, so that's a bug.

Since it's a just a test application I created for a lecture I will present in the office next week I don't mind using dnx ef database 0 and start all over again.
But out of curiosity what should I do if it is a real production application? fiddle with the __EFMigrationsHistory table?

Thank you very much Rowan, I really like what you did with EF 7 and in ASP.NET 5 in general!

@smitpatel
Copy link
Member

Renaming issue is dupe of #3990
Its ordering issue. You can manually edit generated migration to use older name in operations before the rename operation is executed. Sql script is generated from migration file so once migration file is modified it will generate correct script. (i.e. using older names before rename operation is called)

@gdoron
Copy link

gdoron commented Feb 12, 2016

@rowanmiller Should be noted that Type.GetProperties is not available on dnxcore.
Are there alternatives?

@smitpatel
Copy link
Member

@gdoron - try using type.GetRuntimeProperties()

@divega
Copy link
Contributor

divega commented Feb 12, 2016

@gdoron You should probably use code like what we have in DbSetFinder to accomplish this (or even use our internal type in the extension for now).

Anyway, in the interest of keeping this work item actionable for the scaffolding part, I have created a separate issue for using the DbSet property names for tables names without having to write any extra code: #4540.

Consider creating a new issue if you hit any bugs that aren't covered by existing issues 😄

@divega divega changed the title Feature Request: Database RevEng pluralization service support RevEng: Use pluralization and when scaffolding a model from a database Feb 12, 2016
@divega divega changed the title RevEng: Use pluralization and when scaffolding a model from a database RevEng: Use pluralization and singularization when scaffolding a model from a database Feb 12, 2016
@rowanmiller rowanmiller added this to the Backlog milestone Feb 12, 2016
@rowanmiller rowanmiller modified the milestones: 1.0.0, Backlog May 2, 2016
@rowanmiller rowanmiller changed the title RevEng: Use pluralization and singularization when scaffolding a model from a database Reverse Engineer: Use pluralization/singularization when scaffolding a model from a database May 13, 2016
@rowanmiller rowanmiller removed the pri1 label May 13, 2016
@rowanmiller rowanmiller modified the milestones: 1.0.1, 1.0.0 May 13, 2016
@rowanmiller
Copy link
Contributor

Talk with @anpete about using a 3rd party pluralizer

rowanmiller added a commit that referenced this issue Feb 16, 2017
Introduces a new IPluralizer service that is used to singularize entity type names and pluralize DbSet names. The default implementation is a no-op, so this is just a hook where folks can easily plug in their own pluralizer.

I left this separate from the existing CandidateNamingService, which is responsible for cleaning up the table name into a C# friendly identifier.

Here is what it looks like for a developer to hook in their own pluralizer. Our code generation coverage is lacking, so I tested this against Northwind and verified by round tripping data.

```c#
public class Startup
{
    public void ConfigureDesignTimeServices(IServiceCollection services)
    {
        services.AddSingleton<IPluralizer, MyPluralizer>();
    }
}

public class MyPluralizer : IPluralizer
{
    public string Pluralize(string name)
    {
        return Inflector.Inflector.Pluralize(name) ?? name;
    }

    public string Singularize(string name)
    {
        return Inflector.Inflector.Singularize(name) ?? name;
    }
}
```

Resolves #3060
rowanmiller added a commit that referenced this issue Feb 16, 2017
Introduces a new IPluralizer service that is used to singularize entity type names and pluralize DbSet names. The default implementation is a no-op, so this is just a hook where folks can easily plug in their own pluralizer.

I left this separate from the existing CandidateNamingService, which is responsible for cleaning up the table name into a C# friendly identifier.

Here is what it looks like for a developer to hook in their own pluralizer. Our code generation coverage is lacking, so I tested this against Northwind and verified by round tripping data.

```c#
public class Startup
{
    public void ConfigureDesignTimeServices(IServiceCollection services)
    {
        services.AddSingleton<IPluralizer, MyPluralizer>();
    }
}

public class MyPluralizer : IPluralizer
{
    public string Pluralize(string name)
    {
        return Inflector.Inflector.Pluralize(name) ?? name;
    }

    public string Singularize(string name)
    {
        return Inflector.Inflector.Singularize(name) ?? name;
    }
}
```

Resolves #3060
rowanmiller added a commit that referenced this issue Feb 16, 2017
Introduces a new IPluralizer service that is used to singularize entity type names and pluralize DbSet names. The default implementation is a no-op, so this is just a hook where folks can easily plug in their own pluralizer.

I left this separate from the existing CandidateNamingService, which is responsible for cleaning up the table name into a C# friendly identifier.

Here is what it looks like for a developer to hook in their own pluralizer. Our code generation coverage is lacking, so I tested this against Northwind and verified by round tripping data.

```c#
public class MyDesignTimeServices : IDesignTimeServices
{
    public void ConfigureDesignTimeServices(IServiceCollection services)
    {
        services.AddSingleton<IPluralizer, MyPluralizer>();
    }
}

public class MyPluralizer : IPluralizer
{
    public string Pluralize(string name)
    {
        return Inflector.Inflector.Pluralize(name) ?? name;
    }

    public string Singularize(string name)
    {
        return Inflector.Inflector.Singularize(name) ?? name;
    }
}
```

Resolves #3060
@rowanmiller rowanmiller removed this from the 2.0.0 milestone Feb 16, 2017
@rowanmiller rowanmiller reopened this Feb 16, 2017
@rowanmiller
Copy link
Contributor

Clearing up to discuss whether we want to have pluralization baked into the default experience, or whether we are ok with just having the hook to wire in your own.

@maumar maumar closed this as completed in fce49a2 Feb 19, 2017
@divega
Copy link
Contributor

divega commented Feb 19, 2017

@maumar @rowanmiller just to make sure, was this closed intentionally or was there something else we wanted to do?

@maumar
Copy link
Contributor

maumar commented Feb 19, 2017

@divega accident I think I goofed something up with that last commit

@maumar maumar reopened this Feb 19, 2017
@rowanmiller rowanmiller changed the title Reverse Engineering: Use pluralization/singularization when scaffolding a model from a database Reverse Engineering: Enable pluralization/singularization when scaffolding a model from a database Feb 22, 2017
@rowanmiller rowanmiller added this to the 2.0.0 milestone Feb 22, 2017
@rowanmiller rowanmiller added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 22, 2017
@rowanmiller
Copy link
Contributor

Discussed and decided that we are ok with just having the hook for the moment. We'd like to avoid taking a dependency on a pluralization service. We may reconsider in the future if we see folks having issues with using a third party service.

roji added a commit to roji/efcore.pg that referenced this issue Feb 28, 2017
roji added a commit to roji/efcore.pg that referenced this issue Mar 1, 2017
@divega divega modified the milestone: 2.0.0-preview1 May 10, 2017
@SidShetye
Copy link

SidShetye commented Jul 5, 2018

What's the definitive final guide on singularization of entity classes and pluralization of collections? Documentation is empty (as of now) and across the last 3 years there are multiple offered solutions on stackoverflow and github; esp as EF Core has evolved (e.g. dbsets now take the table names i.e. pluralized)

For some background, we're looking at EF Core 2.1 from EF 6. We don't trust migration wizardry anywhere close to our QA or production databases, so use proper SQL scripts to manage DB schema and then reverse engineering/scaffold the model back into code. IMHO since pluralization of collections and singularization of entity classes is standard practice, the default out-of-box product experience should address it.

@ErikEJ
Copy link
Contributor

ErikEJ commented Jul 5, 2018

@SidShetye "EF Core Power Tools" reverse engineer has pluralization

@SidShetye
Copy link

SidShetye commented Jul 10, 2018

@ErikEJ that looks great. We used IDesignTimeServices and IPluralizer with a custom class that pluralized and singularized strings (i.e. acting as an inflector).

My request is have this on by default; as an out-of-box feature that makes EF Core even better. It's literally a few hours of engineering time but adds so much value for folks following the reverse engineering workflow, that the value/invest fraction should be worth it (obv. the EF Core team knows their constraints best).

Plus there are multiple well-licensed implementations (here and here and even your own one here :) ) to accelerate this.

@bricelam
Copy link
Contributor

We talked about adding one as part of #11160. Not sure what we decided...

See also Pluralization in EF Core.

@ajcvickers ajcvickers modified the milestones: 2.0.0-preview1, 2.0.0 Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests