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

Expose Current ConventionSet #5737

Closed
rjperes opened this issue Jun 12, 2016 · 5 comments
Closed

Expose Current ConventionSet #5737

rjperes opened this issue Jun 12, 2016 · 5 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-unknown
Milestone

Comments

@rjperes
Copy link

rjperes commented Jun 12, 2016

As of now, the current ConventionSet is not publicly exposed. It is only possible, AFAIK, to get a pointer to it by using reflection to access the _conventionSet field of the ConventionDispatcher.
In order to make conventions simpler to use, can't you expose the current ConventionSet as a property somewhere?

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Jun 13, 2016

@rjperes The ConvetionSet is not exposed because ModelBuilder assumes that it is readonly. The correct way to change the conventions that will be used is by deriving from the provider-specific implementation of IConventionSetBuilder and adding it to DI, overriding the default one. Since each provider can have a different set of conventions you would need to do it for each provider that will be used in the application.

As an example if you want to replace the RelationshipDiscoveryConvention that runs when an entity type is added for SqlServer, you would implement something like:

public class CustomSqlServerConventionSetBuilder : SqlServerConventionSetBuilder
{
    public CustomSqlServerConventionSetBuilder(
        IRelationalTypeMapper typeMapper, ICurrentDbContext currentContext, IDbSetFinder setFinder)
        : base(typeMapper, currentContext, setFinder)
    {
    }

    public override ConventionSet AddConventions(ConventionSet conventionSet)
    {
        base.AddConventions(conventionSet);

        conventionSet.ModelInitializedConventions.Add(new SqlServerValueGenerationStrategyConvention());

        var relationshipDiscoveryConvention =
            conventionSet.EntityTypeAddedConventions.OfType<RelationshipDiscoveryConvention>().FirstOrDefault();

        var index = conventionSet.EntityTypeAddedConventions.IndexOf(relationshipDiscoveryConvention);
        conventionSet.EntityTypeAddedConventions.RemoveAt(index);
        conventionSet.EntityTypeAddedConventions.Insert(index, new BetterRelationshipDiscoveryConvention());

        return conventionSet;
    }
}

Then add it to DI:

services
.AddEntityFrameworkSqlServer()
.AddScoped<SqlServerConventionSetBuilder, CustomSqlServerConventionSetBuilder>()

This experience will be improved by #214

@rjperes
Copy link
Author

rjperes commented Jun 13, 2016

Well, you didn't explain anything that I didn't knew. The point of this PR was precisely to change that, by giving developers a chance to add conventions at runtime without the need to subclass an infrastructure class.

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Jun 13, 2016

Changing the conventions in OnModelCreating could lead to unexpected results, since some conventions have already run on the entity types discovered through the entity sets on the context type.

@rjperes
Copy link
Author

rjperes commented Jun 13, 2016

So what? That is exactly what happened with custom conventions in EF pre-Core!

@AndriySvyryd
Copy link
Member

The way conventions are applied in EF Core is fundamentally different from EF6.

In EF6 OnModelCreating created a DbModelBuilder with the specified conventions and configurations. Then EF would create a model, apply the configurations, then the data annotations, then the conventions.

However in EF Core the configuration specified in OnModelCreating is applied to the model immediately and appropriate conventions are applied in reaction to the changes, so modelBuilder.Model is kept in-sync with the configuration. This allows to use the current state of the model to apply further configurations.

@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 15, 2022
@ajcvickers ajcvickers modified the milestones: 1.1.0-preview1, 1.1.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-unknown
Projects
None yet
Development

No branches or pull requests

4 participants