ErikEJ / EFCorePowerTools

Entity Framework Core Power Tools - reverse engineering, migrations and model visualization in Visual Studio & CLI
MIT License
2.11k stars 292 forks source link

Support composite database projects #274

Closed Herdo closed 4 years ago

Herdo commented 4 years ago

This issue is a rough idea for reverse engineering a database context from a composite database project.

A composite database project is the aggregate of multiple, smaller and reusable DACPACs. For example, a shared DACPAC contains a table called User, which is used accross multiple applications that access the same database. Instead of defining the table User in every application's database project, the table is provided in the DACPAC. This DACPAC gets referenced by the application database projects.

When reverse engineering the composite application database project, I'd like to include the User table from the shared DACPAC in the same data context.


The only way to do this right now is to deploy the composite database project with all dependencies to a database, and reverse engineer from there.

With this issue, I'd like to suggest to "somehow" skip this deployment step, and directly create a composite database context from the composite database project.

ErikEJ commented 4 years ago

Can you provide a sample VS solution?

Herdo commented 4 years ago

Can you provide a sample VS solution?

Sure, will do soon.

Herdo commented 4 years ago

Here's an example solution: CompositeDatabase.zip

Trying to reverse engineer the composite database project fails for me.

Herdo commented 4 years ago

Note to the ZIP: I didn't expect it to fail in the initial report. So I'd still call this an enhancement than a bug πŸ˜‰

ErikEJ commented 4 years ago

Hmm.. Looks like you can:

If you find any then you can load the reference dacpac, script it, and load it into the "mother" TSqlModel, but it seems quite brittle...

https://docs.microsoft.com/en-us/dotnet/api/microsoft.sqlserver.dac.dacservices.script?view=sql-dacfx-140.3881.1

https://docs.microsoft.com/en-us/dotnet/api/microsoft.sqlserver.dac.model.tsqlmodel.addobjects?view=sql-dacfx-140.3881.1#Microsoft_SqlServer_Dac_Model_TSqlModel_AddObjects_System_String_

Herdo commented 4 years ago

Seems like this is the only supported method by Microsoft as of now. Sadly. @ErikEJ However, I can imagine this workflow would work:

  1. Load the primary DACPAC that's selected in the dialog into a TSqlModel instance primaryModel.
  2. Check the primaryModel has references (somehow) OR check if there are more DACPACs in the same directory as the primary DACPAC.
  3. Load those dependency DACPACs into individual TSqlModel instances dependencyModels.
  4. Iterate through dependencyModels and call dependencyModel.GetObjects(...) for all supported and desired objects.
  5. On each TSqlObject from step 4, call objectToAdd.TryGetAst(...) to export that objects script.
  6. Use primaryModel.AddObjects(...) to add the dependency objects.
  7. Generate classes for primaryModel.

In my opinion, deciding on merging those dependency DACPACs into the composite TSqlModel should happen pretty early. If we can detect dependencies, ask the user if those should be scanned? And for the table selection dialog, somehow present that table dbo.XYZ is from a dependency, not the composite project.

NickFranceschina commented 4 years ago

just a note: I ran into this trying to use the tool for the first time, and my reverse-engineer dialog doesn't show the "dacpac" drop-down at all.. so I thought maybe there was a bug since the Wiki clearly shows it I was about to write an issue but found this one and yes my db proj was a composite, so I refactored it to be a single just to see if that was the issue, then the drop-down appeared and the tool worked as normal

Screen Shot 2020-03-05 at 12 09 36 AM

ErikEJ commented 4 years ago

@NickFranceschina Thanks for the info - as you can see, adding support for this is quite complicated/brittle. Feel free to case a vote (or submit a PR :smile: )

ErikEJ commented 4 years ago

Use this to get all references: https://github.com/GoEddie/Dacpac-References/blob/master/src/DacpacHeaderParser.Tests/ReadHeaderTests.cs

You can use it to add references to a dacpac then re-load that as a TSqlModel, trying to manually add a reference to an already existing model is quite hard

ErikEJ commented 4 years ago

@nickfranceschina what you are reporting actually sounds like an unrelated bug, I will look into it. Are you able to provide a repro project? #362

ErikEJ commented 4 years ago

@Herdo OK, I think I finally figured it out:

Create a method in SqlServerDacpacdatabaseModelFactory = dacpacPath = TryGetReferences(dacpacPath);

1: Recursively look for any external references using https://github.com/GoEddie/Dacpac-References code

2: If there are any, merge all references into a new temporary master (a copy of the selected dacpac), using https://github.com/GoEddie/DacpacMerge code and return new name ortherwise just return original name

Herdo commented 4 years ago

@ErikEJ Sounds like a good idea πŸ‘

ErikEJ commented 4 years ago

@herdo I have implemented initial support, and tested against your sample solution. Could you verify that the concept works? (EF Core 2.0 reverse enginering only, until implementation is tested and complete) - TIA!

Herdo commented 4 years ago

@ErikEJ This is failing on a real-life project for an unknown reason. I'm getting this error at ProjectExtensions.TryBuild(this Project project) on dte.Solution.SolutionBuild.BuildProject(configuration, project.UniqueName, true);: grafik

These errors occur a few hundred times in the output window:

"antlr.NoViableAltForCharException" in Microsoft.SqlServer.TransactSql.ScriptDom.dll
"antlr.NoViableAltException" in Microsoft.SqlServer.TransactSql.ScriptDom.dll
"antlr.MismatchedTokenException" in Microsoft.SqlServer.TransactSql.ScriptDom.dll
"System.Exception" in Microsoft.VisualStudio.Data.Tools.Package.dll

Any idea what this is about? If not, I'll try to figure out something more next week.

ErikEJ commented 4 years ago

No idea. Could be a bug in the .NET Core DacFx library. Please try EF Core 2 reverse engineering. Or share the dacpacs with me privately

ErikEJ commented 4 years ago

Also, I had som build breaks, so please make sure you have build 2.4.15 or later

ErikEJ commented 4 years ago

Actually, looks like I have some publsihing issues, latest daily is currently very old!

ErikEJ commented 4 years ago

OK, finally a proper daily build is available!

Herdo commented 4 years ago

Thanks, I'll check the daily build with a real-life project on Tuesday πŸ‘

Herdo commented 4 years ago

I tried on two real-live database projects. One worked like a charm, with only 2 references. The other one, with about 15 references, failed.

The ReferencedDatabaseProjectDACPAC.DACPAC is in the same directory as the build output of the selected database project, however, something is trying to fetch the DACPAC from the original build output directory from the referenced DACPAC. Instead loading the referenced DACPAC from the correct directory, right next to the DACPAC selected in the dialog: C:\MySolution\MyProject\BIN\OUTPUT\ReferencedDatabaseProjectDACPAC.DACPAC, It’s loading the referenced DACPAC from its original build output directory: C:\SharedSolution\ReferencedDatabaseProject\BIN\DEBUG\ReferencedDatabaseProjectDACPAC.DACPAC.

Not sure if this is a problem with the GOEddie.Dacpac.Reference lib or EFCorePowerTools. Guess I need to debug this? Or do you see the issue?


System.IO.DirectoryNotFoundException: Could not find a part of the path "C:\SharedSolution\ReferencedDatabaseProject\BIN\DEBUG\ReferencedDatabaseProjectDACPAC.DACPAC".
   at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
   at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, Boolean useAsync)
   at MS.Internal.IO.Zip.ZipArchive.OpenOnFile(String path, FileMode mode, FileAccess access, FileShare share, Boolean streaming)
   at System.IO.Packaging.ZipPackage..ctor(String path, FileMode mode, FileAccess access, FileShare share, Boolean streaming)
   at System.IO.Packaging.Package.Open(String path, FileMode packageMode, FileAccess packageAccess, FileShare packageShare, Boolean streaming)
   at GOEddie.Dacpac.References.HeaderParser.GetCustomData()
   at ReverseEngineer20.DacpacConsolidate.DacpacConsolidator.GetAllReferences(String dacpacPath)
   at ReverseEngineer20.DacpacConsolidate.DacpacConsolidator.GetAllReferences(String dacpacPath)
   at ReverseEngineer20.DacpacConsolidate.DacpacConsolidator.GetAllReferences(String dacpacPath)
   at ReverseEngineer20.DacpacConsolidate.DacpacConsolidator.Consolidate(String dacpacPath)
   at ReverseEngineer20.DacpacTableListBuilder.GetTableDefinitions()
   at EFCorePowerTools.Handlers.ReverseEngineerHandler.GetDacpacTables(String dacpacPath) in C:\projects\efcorepowertools\src\GUI\EFCorePowerTools\Handlers\ReverseEngineerHandler.cs:Line 339.
   at EFCorePowerTools.Handlers.ReverseEngineerHandler.<ReverseEngineerCodeFirst>d__3.MoveNext() in C:\projects\efcorepowertools\src\GUI\EFCorePowerTools\Handlers\ReverseEngineerHandler.cs:Line 117.```
ErikEJ commented 4 years ago

Look at the refrences section in the model.xml inside the master .dacpac - I am guessing the path there points to an invalid location?

ErikEJ commented 4 years ago

If the file does not exist there, I guess I could probe the folder where the master dacpac resides?

Herdo commented 4 years ago

Look at the refrences section in the model.xml inside the master .dacpac - I am guessing the path there points to an invalid location?

The reference in model.xml is fine. It's pointing to our DACPAC repository where all DACPACs always exist. πŸ‘

If the file does not exist there, I guess I could probe the folder where the master dacpac resides?

Yeah, I had some fun with DACPAC references myself while developing my SSDTLifecycleExtension. I think the actual issue here is the fact that I reference ReferencedDatabaseProjectDACPAC.DACPAC from my main project directly, as well as one of the referenced DACPACs is referencing the ReferencedDatabaseProjectDACPAC.DACPAC, but from its bin directory. So due to the recursion in DacpacConsolidator.GetAllReferences(string dacpacPath), it's finding the same DACPAC twice, at two or more locations. When one is pointing to the bin\Debug location, it'll fail if that project was not build on the machine.

I'd propose this pattern for DacpacConsolidator:

  1. Keep a dictionary for each unique DACPAC, identified by LogicalName to avoid duplicates (as described above to avoid duplicated).
  2. Check if a file with the LogicalName is in the directory of the main DACPAC, e.g. bin\Debug, bin\Release, bin\Output, or whatever is configured as output directory. If found, add this to the dictionary.
  3. If 2 doesn't work (which should, as all binaries/DACPACs should be copied during the build), check the DACPAC at FileName. Add this to the dictionary using the LogicalName as key.

This should avoid duplicates and allow reverse engineering of more complex database projects and DACPACs.

ErikEJ commented 4 years ago

Just to understand in master.dacpac:

in Reference1.dacpac;

Is that how your (circular) references are set up?

Herdo commented 4 years ago

So we have two different use-cases we apply, both in the single example below. If we need something in Ref1.dacpac, we'll reference it, otherwise we suppress the missing reference.

Then we have C:\MyBusinessSolution\MyBusinessSolution.sln with C:\MyBusinessSolution\PrimaryDatabase.sqlproj:

This project references C:\DacpacRespository\Ref2.dacpac, so that you don't have to build Ref2 locally every time.

Case 1

It DOES references C:\DacpacRepository\Ref1.dacpac, because it's required for the PrimaryDatabase project, e.g. by using a table from Ref1.dacpac.

Case 2

It DOES NOT references C:\DacpacRepository\Ref1.dacpac, because it's not required for the PrimaryDatabase project. It's just a reference that will be needed for Ref2.dacpac.

Conclusion

For case 1, the dictionary approach written in the previous comment should be applied to avoid referencing Ref1.dacpac multiple times. For case 2, when the Ref1.dacpac is not found, because it's neither referenced by the PrimaryDatabase.sqlproj directly (therefore not at bin\output\Ref1.dacpac), nor found in the C:\SharedSolution\Ref1\bin\output\Ref1.dacpac, a missing DACPAC should just be ignored for reverse engineering.

I hope those two cases help πŸ˜…

ErikEJ commented 4 years ago

Seems like the simple solution is to only look for referenced .dacpac files in the folder where the master exists, and if they do not exist there, ignore them.

Herdo commented 4 years ago

Yep, that'd be the easiest solutions, I guess.

ErikEJ commented 4 years ago

No, I meant, just keep the recursive reference collection mechanism, but only attenmpt to load files that are located in the folder of the master dacpac.

Herdo commented 4 years ago

Okay, sounds good.

ErikEJ commented 4 years ago

Fixed in latest daily build

Herdo commented 4 years ago

The recursion does still break the case 2 described above. I'll open a PR for it.

ErikEJ commented 4 years ago

Thanks, otherwise I would need a folder with .dacpac files from you! πŸ˜„