TrackableEntities / EntityFrameworkCore.Scaffolding.Handlebars

Scaffold EF Core models using Handlebars templates.
MIT License
210 stars 53 forks source link

EnableSchemaFolders with namespace #122

Closed audacity76 closed 4 years ago

audacity76 commented 4 years ago

Added a new transformer EntityTypeNameTransformer. This is used to prefix the entity type name with the model namespace. Cleaned up documentation.

As the context namespace can differ from the model namespace I used a namespace alias called Models for referencing the entity types. The EntityTypeNameTransformer uses the IEntityType to get the schema and calls the EntityNameTransformer to get the entity name. Entities are then referenced as Models.{Schema}.{TransformedEntityName}.

Closes #119

tonysneed commented 4 years ago

Hi @audacity76,

Can you tell me why you think a transformer is needed to implement this feature? I would like to propose an approach which does not require adding a transformer. (Perhaps a separate PR could address a need here.)

From what I can tell, you first need to append the schema name to the namespace in each model class. For example (where dbo is the schema name):

namespace ScaffoldingSample.Models.dbo
{
    public partial class Category

I see that you are doing this here, but I don't think it's necessary to add a namespace alias to the model class.

Secondly, you need to prefix each DbSet type argument with the schema name, like so:

public virtual DbSet<dbo.Category> Category { get; set; }

To implement this, update the HbsCSharpDbContextGenerator.GenerateDbSets method to prefix the schema name:

var setPropertyName = _options?.Value?.EnableSchemaFolders == true
    ? $"{entityType.GetSchema()}.{entityType.GetDbSetName()}"
    : entityType.GetDbSetName();

However, to make this work you need to add a namespace alias, because class name collisions are possible. However, they should be set to the schema name, rather than "Models", like so:

using dbo = ScaffoldingSample.Models.dbo;

The tricky part is that there may be more than one alias needed. To accomplish this, you can add a GenerateModelImports method to HbsCSharpDbContextGenerator, in which you add a "model-imports" dictionary. (Make sure to call GenerateModelImports from GenerateClass.)

private void GenerateModelImports(IModel model)
{
    var modelImports = new List<Dictionary<string, object>>();
    var schemas = model.GetScaffoldEntityTypes(_options.Value)
        .Select(e => e.GetSchema())
        .OrderBy(s => s)
        .Distinct();

    foreach (var schema in schemas)
    {
        modelImports.Add(new Dictionary<string, object>
        {
            { "model-import", $"{schema} = {_modelNamespace}.{schema}"}
        });
    }

    TemplateData.Add("model-imports", modelImports);
}

Lastly, update the DbImports.hbs template to add the model imports.

{{#each model-imports}}
using {{model-import}};
{{/each}}
audacity76 commented 4 years ago

Hello @tonysneed,

I created the transformer because there are a lot of places where the HbsCSharpDbEntityGenerator and the HbsCSharpDbContextGenerator need to prefix the type name. The entity name transformer was always called like <IEntityType>.Name. So instead I thought it might be handy to let the transformer decide which properties it needs - so for now it uses the db schema. At last I want to gave the transformer a chance to identify different entities if the same entity (name) exists in 2 different schemas.

I introduced the Models alias namespace to minimize namespace collisions, but sure I can list all schema namespaces as alias as you suggested.

tonysneed commented 4 years ago

First I wanted to say thanks, @audacity76, for your contribution.

Would you be willing to create a new PR with the changes I proposed? (If not, I can implement them.) Then this PR could focus on functionality provided by adding the transformer.

Regarding the Models alias, if you have the same table name in different schemas, wouldn’t a single Models alias still result in a class name collision?

audacity76 commented 4 years ago

@tonysneed, as the schema namespaces depend on the new transformer it may be easier for you to put the 2 tasks in line.

The Models namespaces alias referenced the models base path. So the type reference always included the schema like Models.dbo.SomeTable. But I also wasn't fully satisfied with the hard coded alias.

tonysneed commented 4 years ago

@audacity76 Implemented my recommendations in PR #126. Gave you credit in commit message and release notes.

audacity76 commented 4 years ago

@tonysneed Thank you very much!