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

Accept PDO as a Connection constructor argument #4948

Merged
merged 1 commit into from
Nov 6, 2021

Conversation

morozov
Copy link
Member

@morozov morozov commented Nov 5, 2021

Q A
Type improvement
BC Break no

This patch is a step towards sunsetting Connection::getWrappedConnection(). This method is not part of the driver API, so wrapping a driver connection into a decorator will make this method inaccessible. This is likely already a problem when the Portability middleware is used (#4157) and will become more obvious when the Logging middleware has been introduced (#4941).

Instead of instantiating PDO, the driver connection constructor will now accept it as a dependency. It has the following advantages:

  1. It becomes possible to implement a driver that uses a custom PDO instance and reuse the rest of the PDO driver components (example). This solves the problem of sharing an underlying connection between multiple libraries w/o using a dedicated type-unsafe API.
  2. The pdo_pgsql and pdo_sqlite drivers will no longer break their connection encapsulation by calling Connection::getWrappedConnection() in order to access the underlying PDO instance.

The general idea here is that every driver-level component should accept and wrap its lower-level counterpart, not instantiate it. This will allow for a more clear separation of concerns and, eventually, a cleaner internal API.

A similar change was applied recently in #4894.

@morozov morozov merged commit 7d7b00e into doctrine:3.2.x Nov 6, 2021
@morozov morozov deleted the pdo-constructor-argument branch November 6, 2021 06:37
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants