PHPORM-278 Introduce Connection::getDatabase() and getClient()
#3289
Conversation
Deprecate getMongoDB and get MongoClient
| @@ -12,15 +12,18 @@ | |||
|
|
|||
| use function MongoDB\with_transaction; | |||
|
|
|||
| /** @see https://docs.mongodb.com/manual/core/transactions/ */ | |||
| /** | |||
| * @internal | |||
There was a problem hiding this comment.
Was this class always internal? Just curious if there's a BC concern here.
There was a problem hiding this comment.
We try to do better than Laravel's ManagesTransactions trait, that does not declare its dependencies. This trait is not meant to be used outside of the Connection class (it should be merged in it but we kept Laravel's design).
Technically, that's a breaking change that is easily overcome by adding a getClient method to the class that uses it.
| $this->assertSame($expected, $connection->getMongoClient()); | ||
| } | ||
|
|
||
| public function testLegacyGetMongoDB(): void |
There was a problem hiding this comment.
Do these legacy tests need to expect a deprecation message?
There was a problem hiding this comment.
Something catches deprecation messages and hides them. I couldn't add an assertion.
| @@ -65,9 +68,10 @@ public function __construct(array $config) | |||
|
|
|||
| // Create the connection | |||
| $this->connection = $this->createConnection($dsn, $config, $options); | |||
| $this->database = $this->getDefaultDatabaseName($dsn, $config); | |||
There was a problem hiding this comment.
Is this dynamically assigning a class property, or is this declared on BaseConnection?
Either way, this appears to be the first time it's being used.
There was a problem hiding this comment.
The property is defined in the parent class, it was not set previously.
GromNaN
changed the title
Connection::getDatabase() and getClientConnection::getDatabase() and getClient()
And deprecate getMongoDB and get MongoClient
Fix PHPORM-278
Checklist