Scaffold-DbContext includes "using statement" with model´s namespace #18405
Conversation
Summary of changes - Add a new parameter to the WriteCode method of the ICSharpDbContextGenerator interface to pass the namespace of the models - CSharpDbContextGenerator generates a using statement in the DbContext class if the models are in another namespace - Add two test on CSharpDbContextGeneratorTest Fixes #17839
|
|
| @@ -62,9 +62,10 @@ public class CSharpDbContextGenerator : ICSharpDbContextGenerator | |||
| /// </summary> | |||
| public virtual string WriteCode( | |||
| IModel model, | |||
| string @namespace, | |||
There was a problem hiding this comment.
Should keep old API for provider compatibility.
There was a problem hiding this comment.
Unless this goes to master.
There was a problem hiding this comment.
Wait, is ICSharpDbContextGenerator .Internal too? I feel like it should be
There was a problem hiding this comment.
Too late to make anything internal for a while now...
There was a problem hiding this comment.
It's Internal. This isn't breaking.
|
I wonder if we should consider #3988 when fixing this. The implication there is that the entity types could be in multiple namespaces. Would putting the root model namespace in |
|
@bricelam Are you suggesting that we accept multiple model namespaces in the overload being added here? If so, I agree. |
|
@ajcvickers - Model base namespace would be same but if there are different schemas then there would be many different subnamespaces, all of which should be added to using. So we need a better mechanism to tell DbContextGenerator that these are the namespaces where entityTypes are generated. Probably shelf this for next release. We have already punted it for 3.1 |
|
What I had in mind: This method gets the IModel already, so maybe we add a step before this that populates it with annotations for the namespace of things. This would just foreach through the entity types and add the usings it needs. |
|
@bricelam Entity type names should be namespace-qualified anyway, so do we need other information? Or is this not the case when we get the model for reverse engineering? |
|
Not the case when we get the model for reverse engineering currently, but it could be. I think that's the right change. |
|
Good point. Yes, it would break code generators not expecting it (e.g. every Handlebars-based template) |
|
Given that all this is Internal, I'm OK taking this in 3.1. @ajcvickers thoughts? |
|
|
||
| if (finalContextNamespace != modelNamespace) | ||
| { | ||
| _sb.AppendLine(String.Concat("using ", modelNamespace, ";")); |
There was a problem hiding this comment.
I'd use...
_sb.Append("using ").Append(modelNamespace).AppendLine(";");
Summary of changes
Fixes #17839