TrackableEntities / EntityFrameworkCore.Scaffolding.Handlebars

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

[Bug] Generated entity ToTable and Property in generated OnModelCreating lack table and column mappings when Handlebars transformers are used #188

Closed gpender closed 2 years ago

gpender commented 2 years ago

Problems We are using this library in an ef core database first application and we have some name changes between our database tables and entity names and our column names and entity properties.
We are able to make these mappings changes using the AddHandlebarsTransformers method calls but the fluent code for the context does not correctly add the .ToTable and does not ever add the .AsColumnName.

In addition to this it would be very useful to know the context name when making the transforms against the following methods entityNameTransformer: MapEntityName, constructorTransformer: MapPropertyInfo, propertyTransformer: MapPropertyInfo, navPropertyTransformer: MapNavPropertyInfo, entityFileNameTransformer: MapEntityName

Workarounds We have worked around both of these issues by implementing our own MyHbsCSharpDbContextGenerator with the following code changes:-

Add the IContextTransformationService contextTransformationService to the constructor then execute in the WriteCode override. The contextFileNameTransformer can then store the context locally for use in the other transformers.

public override string WriteCode(IModel model, string contextName, string connectionString, string contextNamespace,
    string modelNamespace, bool useDataAnnotations, bool suppressConnectionStringWarning, bool suppressOnConfiguring)
{
  Check.NotNull(model, nameof(model));
        ContextTransformationService.TransformContextFileName(contextName + ".cs");
        if (!string.IsNullOrEmpty(modelNamespace) && string.CompareOrdinal(contextNamespace, modelNamespace) != 0)
    _modelNamespace = modelNamespace;

  TemplateData = new Dictionary<string, object>();

  if (_options.Value.TemplateData != null)
  {
    foreach (KeyValuePair<string, object> entry in _options.Value.TemplateData)
    {
      TemplateData.Add(entry.Key, entry.Value);
    }
  }

In the GenerateProperty method check for a property name transformation and add the .HasColumnName as necessary.

private void GenerateProperty(IProperty property, bool useDataAnnotations, IndentedStringBuilder sb)
{
  var propertyName =
    EntityTypeTransformationService.TransformPropertyName(property.Name, property.DeclaringType.Name);
  var lines = new List<string>
        {
          $".{nameof(EntityTypeBuilder.Property)}(e => e.{propertyName})"
            //$".{nameof(EntityTypeBuilder.Property)}(e => e.{EntityTypeTransformationService.TransformPropertyName(property.Name, property.DeclaringType.Name)})"
        };
  // Add .HasColumnName Fluent annotation for remapped columns
  if (!propertyName.Equals(property.Name))
  {
    lines.Add($".HasColumnName(\"{property.Name}\")");
  }

In the GenerateTableName method check for a table name transformation in addition to explicitSchema checks

  var explicitTable = explicitSchema || tableName != null && tableName != entityType.GetDbSetName();
  if (tableName != null)
  {
      var overrideName = EntityTypeTransformationService.TransformTypeEntityName(tableName);
      if (!tableName.Equals(overrideName)) explicitTable = true;
  }
tonysneed commented 2 years ago

We are able to make these mappings changes using the AddHandlebarsTransformers method calls but the fluent code for the context does not correctly add the .ToTable and does not ever add the .AsColumnName.

@gpender Are you saying there is a problem with generated code for the the context OnModelCreating method? If so, could you please provide an example?

gpender commented 2 years ago

Hi @tonysneed , I don't see any problems with the generated code unless I need to specify a different entity name for a table and therefore need the .ToTable fluent call, and/or, I need to specify a different property name for a column and therefore need the .HasColumnName fluent call. Does that make sense? It was a few weeks ago and I have context switched since then :) Happy to provide more info if you need me to. Thanks for all your efforts

tonysneed commented 2 years ago

The most recent release includes fixes to generating OnModelCreating. Could you please try it again to see if the same behavior is exhibited? If so, could you please provide an example of the problem you are encountering?

gpender commented 2 years ago

Hi @tonysneed I believe I have the latest version and my request is still valid.

  1. So for the Entities with different table names I need the generated code to look like this:-

            modelBuilder.Entity<ConfiguredPricePlan>(entity =>
            {
                entity.ToTable("PricePlan");

    I added a transformation service for entity type name but this does not seem to be used in the logic for adding the ToTable fluent method. The code I wrote to make this happen is in my original post

  2. For the Columns with different property names I need the generated code to look like this:-

    
                entity.Property(e => e.Tail)
                    .HasColumnName("TailPrefix")

When I search through the code base there is no code that implements HasColumnName.

The other part of my request was the DI of the IContextTransformationService so that the context can be made available when 
using the other transforms:-
entityNameTransformer: MapEntityName,
constructorTransformer: MapPropertyInfo,
propertyTransformer: MapPropertyInfo,
navPropertyTransformer: MapNavPropertyInfo,
entityFileNameTransformer: MapEntityNamethe 

I hope this adds more clarity to my request and thanks again
tonysneed commented 2 years ago

@gpender Thanks for getting back to me with additional info on your enhancement request.

It sounds like there could be two separate PR's:

  1. Map table and column names to different entity and property names
  2. Make IContextTransformationService available to transformers

Let's work on the first PR before considering the second. Would you be able to create a PR for the first with a couple of unit tests -- one for table name and another for column name? You might want to have a look at the the Contributing Guidelines for how to get started. Thank you in advance!

gpender commented 2 years ago

@tonysneed have made my code changes for part 1/2 and added a couple of unit tests and created a pull request. Hope its ok :)

gpender commented 2 years ago

@tonysneed I am now looking at the second part of my request and wanted to clarify why I wanted it. When I was writing the design time transformer class I created a JSON lookup file for my mappings:-

        "Name": "EntitiesOne:Aircraft",
        "NameOverride": "MyAircraft",
        "Mappings": [
            {
                "Name": "Livery",
                "NameMapping": "MyLivery"
            }
        ]

To correctly select the mappings:-

So I need context and so I need the TransformContextFileName method to be called early on in my Transformer class so I can store the context locally. Currently it is only called when the model is generated in the HbsCSharpModelGenerator. Hope this makes sense.

gpender commented 2 years ago

@tonysneed part 2/2 done and existing unit tests all still passing

tonysneed commented 2 years ago

@gpender Created #198 for part 2 of this issue.