Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

PHPORM-278 Introduce Connection::getDatabase() and getClient() #3289

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

Merged
GromNaN merged 2 commits into mongodb:5.x from GromNaN:PHPORM-278
Feb 26, 2025

Conversation

@GromNaN
Copy link
Member

@GromNaN GromNaN commented Feb 24, 2025

And deprecate getMongoDB and get MongoClient

Fix PHPORM-278

Checklist

  • Add tests and ensure they pass

@GromNaN GromNaN added this to the 5.2 milestone Feb 24, 2025
@GromNaN GromNaN requested a review from a team as a code owner February 24, 2025 21:45
@GromNaN GromNaN mentioned this pull request Feb 24, 2025
3 tasks

/** @see https://docs.mongodb.com/manual/core/transactions/ */
/**
* @internal
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this class always internal? Just curious if there's a BC concern here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

jmikola reacted with thumbs up emoji
$this->assertSame($expected, $connection->getMongoClient());
}

public function testLegacyGetMongoDB(): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these legacy tests need to expect a deprecation message?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something catches deprecation messages and hides them. I couldn't add an assertion.

jmikola reacted with thumbs up emoji

// Create the connection
$this->connection = $this->createConnection($dsn, $config, $options);
$this->database = $this->getDefaultDatabaseName($dsn, $config);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The property is defined in the parent class, it was not set previously.

jmikola reacted with thumbs up emoji
@GromNaN GromNaN merged commit 0a80c70 into mongodb:5.x Feb 26, 2025
44 checks passed
@GromNaN GromNaN changed the title (削除) PHPORM-278 Introduce Connection::getDatabase() and getClient (削除ここまで) (追記) PHPORM-278 Introduce Connection::getDatabase() and getClient() (追記ここまで) Feb 26, 2025
@GromNaN GromNaN deleted the PHPORM-278 branch February 26, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alcaeus alcaeus alcaeus left review comments

@jmikola jmikola jmikola approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /