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

[Bug] GenerateManyToMany produces wrong code when using AddHandlebarsTransformers #216

Open
Herdo opened this issue Aug 9, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@Herdo
Copy link

Herdo commented Aug 9, 2022

Describe the bug
HbsCSharpDbContextGenerator.GenerateManyToMany does not respect configured design time service transformations (Microsoft.EntityFrameworkCore.Design.IDesignTimeServices.ConfigureDesignTimeServices). When AddHandlebarsScaffolding is configured with propertyTransformer, wrong IndexProperty<T> values are configured, basically ignoring AddHandlebarsScaffolding.

As example, the generated index property may look like this: j.IndexerProperty<int>("UserId").HasColumnName("UserId");

This wrong (or rather missing) transformation is causing a runtime issue:

The relationship from 'UserSubscription (Dictionary<string, object>)' to 'User' with foreign key properties {'UserId' : int} cannot target the primary key {'UserId' : InternalUserId} because it is not compatible. Configure a principal key or a set of foreign key properties with compatible types for this relationship.

Expected behavior
The configured EntityPropertyInfo of the propertyTransformer in AddHandlebarsTransformers would also transform the type of the generated IndexProperty<T>.
For example, for properties matching the name UserId map to the type MyNamespace.InternalUserId, the index property should be generated as IndexProperty<MyNamespace.InternalUserId>.

To Reproduce

  1. Implement a IDesignTimeServices:
    serviceCollection.AddHandlebarsTransformers(propertyTransformer: p =>
    {
        // Map to strong types
        if (p.PropertyName.Contains("UserId"))
            return new EntityPropertyInfo("MyNamespace.InternalUserId", p.PropertyName);
        // Use default
        return new EntityPropertyInfo(p.PropertyType, p.PropertyName);
    });
  2. Create a database with three tables:
    • Table User with column UserId as primary key
    • Table Subscription with column SubscriptionId as primary key
    • Table UserSubscription with columns UserId and SubscriptionId as combined primary key
    • Add foreign keys to table UserSubscription
  3. Scaffold from the database with handlebars.
  4. Execute any logic using the scaffolded DbContext.

Make My Life Easier
I'll add this:
If possible, create a pull request that contains a failing unit test.
Otherwise, use the sample/ScaffoldingSample project to reproduce the bug.

@Herdo Herdo added the bug Something isn't working label Aug 9, 2022
@Herdo
Copy link
Author

Herdo commented Aug 9, 2022

@tonysneed If I should provide a pull request with a failing unit test, I'd the three tables [dbo].[Employee], [dbo].[Territory] and [dbo].[EmployeeTerritories] to the Northwind example in the test project. This would have some side effects on the other tests as well. Would that be OK? Otherwise, I'd use the sample/ScaffoldingSample for reproducing the bug.

@tonysneed
Copy link
Contributor

@Herdo Thanks for catching this. You can use the sample/ScaffoldingSample to repro the bug. Cheers.

Herdo added a commit to Herdo/EntityFrameworkCore.Scaffolding.Handlebars that referenced this issue Aug 10, 2022
@Herdo
Copy link
Author

Herdo commented Aug 10, 2022

@tonysneed Provided the sample in the PR.

One further thing I noticed while creating the sample:
In my repository where I found the issue, the IndexerProperty<T> was written twice, once for each column in the many-to-many table, e.g.:

j.IndexerProperty<int>("EmployeeId").HasColumnName("EmployeeID");

j.IndexerProperty<int>("TerritoryId").HasColumnName("TerritoryID");

In the sample project, only one column was generated:

j =>
{
    j.HasKey("EmployeeId", "TerritoryId").HasName("PK_dbo.EmployeeTerritories");

    j.ToTable("EmployeeTerritories");

    j.IndexerProperty<string>("TerritoryId").HasMaxLength(20);
});

My guess is, that only columns that deviate from the standard are added as IndexerProperty, and that standard columns are implicit.

When the propertyTransformer is applied, maybe additional columns have to be added that are currently not generated.
In the example I provided in the PR, I'd expect to see a j.IndexerProperty<EmployeeId>("EmployeeId"); to be generated, because the type deviates from the standard.

@johnwc
Copy link

johnwc commented Sep 29, 2022

Is there a way to tell EF that a table is a many-to-many relationship, that has additional columns? Our many-to-many table has a created column that stores the date when it was created. But it causes EF to not scaffold it as a many-to-many, but rather one-to-many-to-one.

@jbaig
Copy link

jbaig commented Oct 10, 2022

Might be unrelated, but since the Many to Many properties in the scaffolding are nav properties, shouldn't these be using the navPropertyTransformer rather than the entity names?

sb.AppendLine($"{EntityLambdaIdentifier}.{nameof(EntityTypeBuilder.HasMany)}(d => d.{EntityTypeTransformationService.TransformTypeEntityName(skipNavigation.Name)})");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants