-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
RevEng: Refactor to split schema acquisition from more common functionality #2713
Comments
@bricelam re the last point, do we have a bug for reverse engineering for SQLite that we can talk about in triage? The one that was closed today was specifically for UWP projects, and I couldn't find one for SQLite. |
Created #2721 |
@ErikEJ Please note this might change your SQLCE provider too (make it simpler, I hope). I'm planning on having an example checked in soon - but basics are there's a new abstract base class, ReverseEngineeringMetadataModelProvider, which does all the conversion from the relational IModel to the enhanced IModel used for code-gen. You should simply need to override the method ConstructRelationalModel() and produce an IModel that directly represents the DB i.e. has entities (= tables), properties (= columns) and foreign keys. After that ReverseEngineeringMetadataModelProvider takes over. However there will be requirements on the way the relational IModel is set up (what annotations we expect etc). I'll list them when I have the complete list. |
Sounds great, will save me 600 lines of code, I did not make any significant changes anyway: https://github.com/ErikEJ/EntityFramework7.SqlServerCompact/blob/master/src/Design40/ReverseEngineering/SqlCeMetaDataModelProvider.cs |
Why is this line actually neccessary? |
@ErikEJ re that line - we define IsStoreGenerated in the query at https://github.com/aspnet/EntityFramework/blob/dev/src/EntityFramework.SqlServer.Design/ReverseEngineering/Model/TableColumn.cs#L26. Given the way that query is defined at the moment the line at https://github.com/aspnet/EntityFramework/blob/dev/src/EntityFramework.SqlServer.Design/ReverseEngineering/SqlServerMetadataModelProvider.cs#L616 seems unnecessary. But I think I'd rather remove it from the query and keep it (and especially the comment below) here. Either way will work. |
Fixed with commit 5c30958 |
Currently the reverse engineering provider for SQL Server contains the logic that retrieves the schema information from the database into an IModel and also the logic that translates that into an enriched IModel containing entities, relationships and annotations that is ready for the code-gen phase.
Ideally, providers should only need to implement the initial schema acquisition and could delegate the rest to common functionality in EF.
One proposal to address this is to lift the common functionality into base relational provider classes. This would allow the providers to still override the behaviors that they wish to. Creating a reverse engineering provider for a database in which schema acquisition is fundamentally different from SQL Server should help polish such design.
The text was updated successfully, but these errors were encountered: