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

Feature Request - Schema based folders feedback #1075

Closed
chassq opened this issue Aug 17, 2021 · 18 comments
Closed

Feature Request - Schema based folders feedback #1075

chassq opened this issue Aug 17, 2021 · 18 comments
Labels
enhancement New feature or request reveng

Comments

@chassq
Copy link

chassq commented Aug 17, 2021

First of all thank you for #158 and PR #981. This is really great! A few feedback comments:

  1. Would it be possible to add in the model namespace the folder associated (e.g. in folder Lib.Sql/Model/schema1 the namespace for the model would be Lib.Sql.Model.schema1)?
  2. Would it be possible to break out configuration files (when separate) into schema folder/namespaces as well?

Steps to reproduce

Run EF Core Power Tools Reverse Engineer

Further technical details

EF Core Power Tools version: 2.5.692

Database engine: Azure SQL

Visual Studio version: Visual Studio 2019 16.10.4

@karamizrak
Copy link

So, how can I add schema information to this entity namespace?

@ErikEJ
Copy link
Owner

ErikEJ commented Mar 16, 2022

@karamizrak not implemented - do aPR?

@ErikEJ
Copy link
Owner

ErikEJ commented Nov 23, 2022

@georgeemr how does that relate to this issue??

But there are plenty of renaming features, just read the wiki

@aterbo
Copy link

aterbo commented Jan 24, 2023

I'd add another vote for separating schemas into different namespaces. Ideally, that approach would avoid naming collisions and the automatic numbering at the end of classes.

Currently, if I have two tables of the same name in different schemas like [Environmental].[Project] and [Operations].[Project], the tool will create Project.cs and Project1.cs (separated into different folders, if desired).

What I would love would be to get them in two different namespaces, like Project in namespace app.Environmental and Project in namespace app.Operations.

@ErikEJ
Copy link
Owner

ErikEJ commented Jan 24, 2023

@aterbo PRs accepted

rasputino added a commit to rasputino/EFCorePowerTools that referenced this issue May 25, 2023
@rasputino
Copy link
Contributor

I didn't test it (I don't know how, I'm not used to develop VS extensions). But... ¿something like this could be a solution?:
rasputino@aa26e45

@ErikEJ
Copy link
Owner

ErikEJ commented May 25, 2023

@rasputino How will that affect the namespaces in the DbContext?

@rasputino
Copy link
Contributor

All the namespaces should be in the DbContext, and in the rest of the model classes. Finally I could test it and make some improvements. But still needs to be improved. The changes have been made here:
d585d12

I added a ScaffoldingPostgreSqlMultiSchemaTester project in the test solution to test it with the AdventureWorks database (that already exists in the repository).

The biggest change has been made in the src/GUI/RevEng.Core.60/ReverseEngineerScaffolder.cs file where I added a AppendSchemaNamespace that replaces the namespace and appends the needed usings.

If you think that this is the way to add the feature I could continue modifying the interface to add this as an option. And I'll try to know why in scaffoldedModel.AdditionalFiles (https://github.com/rasputino/EFCorePowerTools/blob/d585d12b1451958305d8cd212c23aeedddb04ee1/src/GUI/RevEng.Core.60/ReverseEngineerScaffolder.cs#L323) the model class Productdocument doesn't exist.

@ErikEJ
Copy link
Owner

ErikEJ commented Jun 6, 2023

the model class Productdocument doesn't exist.

@rasputino Maybe because it is a many to many table?

@rasputino
Copy link
Contributor

It was my fault, this table was from another AdventureWorks version I tested in a PostgreSql server and I forgot to delete it.

I added UseSchemaNamespaces as an experimental option in the UI and the code to not interfere with the UseSchemaFolders options. Now are compatible and usable separately.

@ErikEJ If you agree I can do the pull request.

master...rasputino:EFCorePowerTools:master

@ErikEJ
Copy link
Owner

ErikEJ commented Jun 6, 2023

I have a few comments/questions but suggest you ho ahead and create a PR. Amazing effort, thanks

@aterbo
Copy link

aterbo commented Jun 7, 2023

@rasputino Thanks for pulling that together! I've been monitoring this request, but didn't have the skill or bandwidth to work on it myself.

@CZEMacLeod
Copy link

I love where this is going but I do have a couple of thoughts.

I think there needs to be some mechanism for remapping and/or adjusting the schema names (I know there is a schema suffix just now but don't feel that is sufficient).
e.g. I have schemas like c_CustomerName_Project, as well as SubSystem and would need mappings like

  • c_CustomerName_Project -> CustomerName.Project.
  • SubSystem -> RootNamespace.Core.SubSystem

Obviously, I could do some of this manually, and any form of schema handling is good, but I think it needs a little more for my use case (and probably others too).

The issue mentioned where you get clashes between the schema name and the class names could probably be mitigated by using fully qualified type names, instead of adding usings - which would also be needed if you have a conflicting entity name and the namespaces for both were referred to in the same class.

@ErikEJ
Copy link
Owner

ErikEJ commented Jun 7, 2023

@CZEMacLeod yeah, it is not a coincidence that this has not been implemented yet. 😉

@ErikEJ
Copy link
Owner

ErikEJ commented Jun 7, 2023

@rasputino I think this needs some more design. I am putting your PR on hold for now.

@rasputino
Copy link
Contributor

@CZEMacLeod That would be a great feature, but in my opinion this first step covers most use cases. I think that in your case these namespaces are easily 'renameables' with this experimental new feature, now that the full namespace is different in each class.

@ErikEJ Well, you'll let me know if there's anything I can do to help.

@ErikEJ
Copy link
Owner

ErikEJ commented Jun 8, 2023

@rasputino Let me have a closer look and then merge, so we can collect some user feedback

ErikEJ pushed a commit that referenced this issue Jun 12, 2023
* Example change for schema based folder feedback (#1075)

* Creating scaffolding tester for multi schema databases.

* Using AdventureWorks2014.dacpac in ScaffoldingPostgreSqlMultiSchemaTester

Using AdventureWorks2014.dacpac from Test\NUnitTestCore\dacpac\AdventureWorks2014 in ScaffoldingPostgreSqlMultiSchemaTester

* Failed test of ScaffoldingPostgreSqlMultiSchemaTester

* New AppendSchemaNamespace method to add namespaces in code files

* Removing ProductDocument.cs generated in an old version

* Rescaffolding of AdventureWorks test with new suffix

* Added UseSchemaNamespaces as an experimental option

* Fixing code analyzer suggetions

* Adding use-schema-namespaces-preview to the json schema file

* Untabify use-schema-namespaces-preview option in json schema samples files

* eslint json format correction of files efcpt-config.json and efcpt-schema.json

* Using complete schemas list to create usings in SqlServerRoutineScaffolder and derivates

---------

Co-authored-by: Pablo León <[email protected]>
@ErikEJ
Copy link
Owner

ErikEJ commented Jun 12, 2023

Schema namespaces added in latest daily

@ErikEJ ErikEJ closed this as completed Jun 12, 2023
ErikEJ pushed a commit that referenced this issue Jun 20, 2023
…namespaces (#1838)

* Example change for schema based folder feedback (#1075)

* Creating scaffolding tester for multi schema databases.

* Using AdventureWorks2014.dacpac in ScaffoldingPostgreSqlMultiSchemaTester

Using AdventureWorks2014.dacpac from Test\NUnitTestCore\dacpac\AdventureWorks2014 in ScaffoldingPostgreSqlMultiSchemaTester

* Failed test of ScaffoldingPostgreSqlMultiSchemaTester

* New AppendSchemaNamespace method to add namespaces in code files

* Removing ProductDocument.cs generated in an old version

* Rescaffolding of AdventureWorks test with new suffix

* Added UseSchemaNamespaces as an experimental option

* Fixing code analyzer suggetions

* Adding use-schema-namespaces-preview to the json schema file

* Untabify use-schema-namespaces-preview option in json schema samples files

* eslint json format correction of files efcpt-config.json and efcpt-schema.json

* Using complete schemas list to create usings in SqlServerRoutineScaffolder and derivates

* Fix: Issue #1837. Adding namespaces to dbcontext directly from the tables namespaces

---------

Co-authored-by: Pablo León <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request reveng
Projects
None yet
Development

No branches or pull requests

6 participants