Fix compatibility issues with doctrine/orm package version 3

[lxix]

Hey,

This PR is aiming at doctrine/orm 3 support, while dropping version 2. (Similar to #460 )

Hope you like it, suggestions welcomed!

[bartmcleod]

Why would you replace explicit function arguments by an array that leaves people guessing what is intended?

[bartmcleod]

Is it a requirement of the updated components?

[lxix]

Yeah, this is how it's done in the super class. This breaks compatibility with v2. https://github.com/doctrine/orm/blob/c9557c588b3a70ed93caff069d0aa75737f25609/src/EntityRepository.php#L138

[bartmcleod]

And then need to more lines to extract what was already there?

[lxix]

Sadly, this is the workaround (mentioned previously)

[bartmcleod]

mixed as a return type doesn't add much value, does it?

[lxix]

Same thing here. The way doctrine/orm v3 handles this forces us to adapt. https://github.com/doctrine/orm/blob/c9557c588b3a70ed93caff069d0aa75737f25609/src/Internal/Hydration/AbstractHydrator.php#L233

[bartmcleod]

@lxix Can you solve the conflicts maybe, it seems I have merged a PR that has a similar goal, but does not go as far as you do and your solution might be better... Please have a look at my comments as well, maybe I just don't get everything you want to do.

[lxix]

@bartmcleod Thanks for you feedback, i'll look into it once I have free time again.

Update: Done, conflicts resolved

[bartmcleod]

@lxix Thanks for your explanations and updates. I have ran the Github action. As you can see, they failed. The next time you push, the action will run automatically, so it will be easier for you to verify that it is correct. I hope you will find the time to fix the errors.

[bartmcleod]

@lxix I have been able to fix 26 of the 27 errors, in case you have not started yet on those

[bartmcleod]

@lxix The last remaining error seems to be internal to one of the Doctrine packages. We can probably not fix that, unless we fix that package. So we could either skip the import test until that is fixed or remain stuck, unless you know of another solution maybe. Could be it is possible to configure an EntityManagerProvider and then it might be magically injected...

[lxix]

@bartmcleod I've replaced the deprecated CreateSchemaDoctrineCommand and DropSchemaDoctrineCommand with the recommended ones. Also, maybe this way we can get EntityManagerProvider out of the container. Could you approve the workflow for me as it did not run automatically?

[bartmcleod]

@lxix I pulled the changes and ran the tests locally, it now gives

There was 1 error:

1) Lexik\Bundle\TranslationBundle\Tests\Command\ImportTranslationsCommandTest::testExecute
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: The "doctrine.orm.command.entity_manager_provider" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.

[bartmcleod]

@lxix I managed to fix it. It looks a bit clunky, but the test is happy, please test it once more in the UI and the commands and if you are happy, let me know and I will merge it. I must create a v7.0.3 tag on master first to have a fixed point before this bc break.

[lxix]

@bartmcleod Good news! Thanks for your time and assistance

[bartmcleod]

@lxix You are most welcome, thanks for starting this improvement and your persistence!

[bartmcleod]

I just saw we already have a 7.1 tag, so we can safely proceed with the merge when you are happy and tag it v7.2

[lxix]

I ran a few quick manual tests on symfony 7.1 and doctrine/orm 3.3.2 (this is the latest I have right now). UI functionality seems OK. I will test it with 7.2 in the near future.

[bartmcleod]

Ah, yes, I was under the impression that I was using 7.2, but in fact it is 7.1.5. I also see no errors in the UI and the commands.