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

Support doctrine/orm v3 + doctrine/dbal v4 #78

Closed
wants to merge 12 commits into from
Closed

Support doctrine/orm v3 + doctrine/dbal v4 #78

wants to merge 12 commits into from

Conversation

W0rma
Copy link
Contributor

@W0rma W0rma commented Feb 8, 2024

closes Codeception/module-doctrine#1

closes Codeception/module-doctrine#4

I cherry-picked the commits from #73 (fixes a deprecation when using ORM v2) and #76 (runs tests with PHP 8.2 and 8.3).

@@ -568,6 +565,10 @@ public function testOneToManyRecursiveEntityCreation()

public function testHaveFakeRepository()
{
if (version_compare(InstalledVersions::getVersion('doctrine/orm'), '3', '>=')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

EntityManager::$repositoryFactory was made readonly in ORM v3.

@W0rma W0rma changed the title Support doctrine/orm v3 Support doctrine/orm v3 + doctrine/dbal v4 Feb 8, 2024
// Do not call $reflectedRepositoryFactory->isReadOnly() directly because
// phpstan will complain about a non-existing method when using PHP 8.0.
// isReadOnly() is available as-of PHP 8.1.
if ($reflectedRepositoryFactory->hasMethod('isReadOnly') && $reflectedRepositoryFactory->getMethod('isReadOnly')->invoke(null)) {
Copy link
Contributor Author

@W0rma W0rma Feb 8, 2024

Choose a reason for hiding this comment

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

EntityManager::$repositoryFactory was made readonly in ORM v3.

As a consequence haveFakeRepository() does not work for ORM v3 anymore.

@W0rma
Copy link
Contributor Author

W0rma commented Feb 12, 2024

@Naktibalda @TavoNiievez Please let me know if I can help to move this PR forward. It would be great to have support for DBAL v4 and ORM v3.

I have tested the changes in two of my projects and they seem to work fine.
I tested the changes using ORM v2 and v3.

The PRs #73 and #76 can be closed as soon as this PR has been merged.

@TavoNiievez
Copy link
Member

@W0rma For my part, looking very superficially at the changes.

It seems to me that we should not replace annotations by attributes in all tests. Mapping with annotations is still valid so we should have at least 1 test with them to verify that they still work.

Then, we don't need PHP 8.0 support anymore, since that version is no longer officially supported. So, instead of you creating compatibility logic for PHP 8.0 simply remove its support from composer.json and main.yml.

Finally, I would not want this PR to be released in this repository if it is merged. @ThomasLandauer already talked several times about this and each of those times I agreed with him regarding 'does it make sense to have a module that supports Doctrine 3 but is called module-doctrine2?'. New repositories had to be created for other modules to remove the version number from their name and the same should happen here.

cc. @Naktibalda

@ThomasLandauer
Copy link
Member

Indeed, since Doctrine ORM 3 is out now, renaming the module from "Doctrine2" to "Doctrine" is more urgent now. Here's the issue you're referring to: Codeception/module-doctrine#27

@W0rma
Copy link
Contributor Author

W0rma commented Feb 15, 2024

@TavoNiievez

It seems to me that we should not replace annotations by attributes in all tests. Mapping with annotations is still valid so we should have at least 1 test with them to verify that they still work.

Done. Tests now use attributes with ORM v3 and annotations with ORM v2.

Then, we don't need PHP 8.0 support anymore, since that version is no longer officially supported. So, instead of you creating compatibility logic for PHP 8.0 simply remove its support from composer.json and main.yml.

Done. PHP 8.0 has been dropped.

Finally, I would not want this PR to be released in this repository if it is merged.

Personally, I don't see a problem with this repository supporting ORM v2 and ORM v3.
Do you now who are the maintainers of this repository?

According to Codeception/Codeception#6733 @Naktibalda does not plan to make bigger contributions anymore. I really hope that there is someone who can care about making this repository compatible with ORM v3.

@DavertMik
Copy link
Member

@W0rma do you want to be maintainer of this repository? 😅

@TavoNiievez
Copy link
Member

A repository has been created that does not include the version number in its name:
https://github.com/Codeception/module-doctrine

I have moved this PR to that repository and already merged it:
Codeception/module-doctrine#26

I will close the PR to avoid duplicity, it only remains to rename the Doctrine2 class to Doctrine and update the references in the documentation to the new repository, when this is done version 3.1.0 of codeception/module-doctrine can be released.

@W0rma W0rma deleted the orm3 branch February 19, 2024 04:27
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

Successfully merging this pull request may close these issues.

Deprecation warning: EM->connect public usage Support doctrine/orm v3
6 participants