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

static CreateModel does not call to a partial implementation #388

Closed
StevenBonePgh opened this issue Mar 6, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@StevenBonePgh
Copy link
Contributor

commented Mar 6, 2018

Similar to the instance method OnModelCreating() there should likely be a call to a partial implementation of the static CreateModel() if MakeClassesPartial is enabled. Perhaps static partial void OnCreateModelPartial(System.Data.Entity.DbModelBuilder modelBuilder, string schema);?

I noticed this oversight when I needed to manually add a few entities in my partial context class to do Table Splitting. Thanks to your configurable and extensible template generated code, it was such a breeze to to this!

@sjh37 sjh37 added the enhancement label Mar 6, 2018

@sjh37

This comment has been minimized.

Copy link
Owner

commented Mar 6, 2018

Trouble is, it's static.
I actually did have this a while back, but took it out as there were lots of complaints about having to create a partial db context whenever they reverse engineered a database and set MakeClassesPartial = true:

// Users would have to create
public partial class MyDbContext
{
    static System.Data.Entity.DbModelBuilder CreateModelPartial(
        System.Data.Entity.DbModelBuilder modelBuilder, string schema)
    {
        // todo
        return modelBuilder;
    }
}

I can't stub it out by declaring:

static System.Data.Entity.DbModelBuilder CreateModelPartial(System.Data.Entity.DbModelBuilder modelBuilder, string schema);

as static methods must have a body. Therefore devs would be forced to create a partial db context as above. I also shouldn't generate the partial db context for them either, because if you save the .tt file, it would get overwritten...

I think the best option here would be flag indicating if you want a call to CreateModelPartial(modelBuilder, schema).

Thoughts?

@AndyCadley

This comment has been minimized.

Copy link

commented Mar 6, 2018

A static method has to have a body. But a partial static method does not. It should work by just doing:

partial static System.Data.Entity.DbModelBuilder CreateModelPartial(System.Data.Entity.DbModelBuilder modelBuilder, string schema);

Anyone who then wants to provide an implementation simply needs to declare the corresponding partial method with a body and anyone who doesn't need it can let the compiler optimize out the partial stub.

@StevenBonePgh

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2018

Confirming what @AndyCadley said, If I add the following to the generated context:

//Added this new
static partial void OnCreateModelPartial(System.Data.Entity.DbModelBuilder modelBuilder, string schema);
//existing generated code
public static System.Data.Entity.DbModelBuilder CreateModel(System.Data.Entity.DbModelBuilder modelBuilder, string schema)
{
  //omit first part of generated code, add following line before the return statement
  OnCreateModelPartial(modelBuilder, schema);
  return modelBuilder;
}

It compiles just fine (VS 2017, .NET 4.5.2) and at runtime behaves properly, even if the partial class implementation does not exist at all, and also if the partial method implementation is omitted entirely.

//Compiles with or without this in the partial implementation, and even without a partial class implemented.
static partial void OnCreateModelPartial(System.Data.Entity.DbModelBuilder modelBuilder, string schema)
{
  modelBuilder.Configurations.Add(new MyMap(schema));
}

I actually didn't know partial was possible for static methods either until I tried it before opening this issue.

@sjh37 sjh37 closed this in eb926d5 Aug 2, 2018

sjh37 added a commit that referenced this issue Aug 2, 2018

Merge pull request #454 from StevenBonePgh/master
#388 static CreateModel can call to a partial implementation. Thanks to StevenBonePgh.
@sjh37

This comment has been minimized.

Copy link
Owner

commented Oct 2, 2018

Released v2.37.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.