-
Notifications
You must be signed in to change notification settings - Fork 53
EnableSchemaFolders with namespace #122
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
EnableSchemaFolders with namespace #122
Conversation
… entity type. Cleaned up documentation.
|
|
||
| if (!string.IsNullOrEmpty(modelNamespace) && string.CompareOrdinal(contextNamespace, modelNamespace) != 0) | ||
| // add base model namespace if it differs from the context namespace OR if model namespace is distributed into schema folders | ||
| if (!string.IsNullOrEmpty(modelNamespace) && string.CompareOrdinal(contextNamespace, modelNamespace) != 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add parenthesis around
string.CompareOrdinal(contextNamespace, modelNamespace) != 0 || _options?.Value?.EnableSchemaFolders == true
Don't want a null or empty modelNamespace set
|
Hi @audacity76, Can you tell me why you think a transformer is needed to implement this feature? I would like to propose an approach which does not require adding a transformer. (Perhaps a separate PR could address a need here.) From what I can tell, you first need to append the schema name to the namespace in each model class. For example (where namespace ScaffoldingSample.Models.dbo
{
public partial class CategoryI see that you are doing this here, but I don't think it's necessary to add a namespace alias to the model class. Secondly, you need to prefix each public virtual DbSet<dbo.Category> Category { get; set; }To implement this, update the var setPropertyName = _options?.Value?.EnableSchemaFolders == true
? $"{entityType.GetSchema()}.{entityType.GetDbSetName()}"
: entityType.GetDbSetName();However, to make this work you need to add a namespace alias, because class name collisions are possible. However, they should be set to the schema name, rather than "Models", like so: using dbo = ScaffoldingSample.Models.dbo;The tricky part is that there may be more than one alias needed. To accomplish this, you can add a private void GenerateModelImports(IModel model)
{
var modelImports = new List<Dictionary<string, object>>();
var schemas = model.GetScaffoldEntityTypes(_options.Value)
.Select(e => e.GetSchema())
.OrderBy(s => s)
.Distinct();
foreach (var schema in schemas)
{
modelImports.Add(new Dictionary<string, object>
{
{ "model-import", $"{schema} = {_modelNamespace}.{schema}"}
});
}
TemplateData.Add("model-imports", modelImports);
}Lastly, update the DbImports.hbs template to add the model imports. |
|
Hello @tonysneed, I created the transformer because there are a lot of places where the I introduced the |
|
First I wanted to say thanks, @audacity76, for your contribution. Would you be willing to create a new PR with the changes I proposed? (If not, I can implement them.) Then this PR could focus on functionality provided by adding the transformer. Regarding the Models alias, if you have the same table name in different schemas, wouldn’t a single Models alias still result in a class name collision? |
|
@tonysneed, as the schema namespaces depend on the new transformer it may be easier for you to put the 2 tasks in line. The |
|
@audacity76 Implemented my recommendations in PR #126. Gave you credit in commit message and release notes. |
|
@tonysneed Thank you very much! |
Added a new transformer
EntityTypeNameTransformer. This is used to prefix the entity type name with the model namespace. Cleaned up documentation.As the context namespace can differ from the model namespace I used a namespace alias called
Modelsfor referencing the entity types. TheEntityTypeNameTransformeruses theIEntityTypeto get the schema and calls theEntityNameTransformerto get the entity name. Entities are then referenced asModels.{Schema}.{TransformedEntityName}.Closes #119