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

slowly remove doctrine hard dependency #142

Closed
pounard opened this issue Apr 17, 2024 · 3 comments
Closed

slowly remove doctrine hard dependency #142

pounard opened this issue Apr 17, 2024 · 3 comments
Milestone

Comments

@pounard
Copy link
Member

pounard commented Apr 17, 2024

OK, I have a plan, but we need to discuss it.

First, check #141 which unhardcode doctrine/dbal in many places.

Then, if we continue on this path, we need to remove the Doctrine\DBAL\Connection everywhere we use it, and replace it using MakinaCorpus\QueryBuilder\Bridge\Bridge instead. That's the easy one.

Now we are going to have one problem: backuppers and restorers are using Doctrine\Persistence\ManagerRegistry in order to create instances depending upon the database vendor. What I propose here would be:

  • Create a custom ConnectionConfig class, with all properties we need (name, username, password, host, port, vendor name, etc...).
  • Create a interface ConnectionRegistry { getSession(string $name): Bridge; getConfig(string $name): ConnectionConfig; } of our own, which would create the MakinaCorpus\QueryBuilder\Bridge\Bridge depending upon the ConnectionConfig internals.
  • For the Symfony bundle, we can implement this registry directly using the Doctrine\Persistence\ManagerRegistry and live with it, it's OK.
  • Use this custom registry in all our factories, in order to fetch either database session (for anonymizers for example) or the config object (for backkupers and restorers).

By doing that, we would have no DBAL hardcoded dependency left, only the glue code for Symfony.

In the long term, future: we can implement a custom one based upon a configuration file, which would use doctrine to actually create the connection, but in the future this would help in order create a standalone docker container or package.

@SimonMellerin
Copy link
Member

I totally agree with this!

@SimonMellerin
Copy link
Member

For the record, here is connection params from Doctrine/Dbal/Connection: https://github.com/doctrine/dbal/blob/7fb00fdf7091a3ab0a4b6d0eed12170b4c7c2aa8/src/DriverManager.php#L46

@pounard
Copy link
Member Author

pounard commented Apr 23, 2024

Done !

@pounard pounard closed this as completed Apr 23, 2024
@pounard pounard added this to the 1.2.0 milestone May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants