BUGFIX: Prevent premature connection to database in PersistenceManager::persistAllowedObjects

[mhsdesign]

For every simple GET request persistAllowedObjects is called via the Package.php in flow. This results in building a db connection, even if there is nothing to do.

We first check if the entityManager is open and if the entity manager has not been used (something is heavily cached via middle-ware or sth) flows objectmanagement retrieves the entity manger.

Now in this retrieval process - even though doctrines connection is lazy itself - the connection will be made forcefully see \Neos\Flow\Persistence\Doctrine\EntityManagerFactory::create line 120 (

flow-development-collection/Neos.Flow/Classes/Persistence/Doctrine/EntityManagerFactory.php

Line 120 in 11e2348

$connection->connect();

) or

flow-development-collection/Neos.Flow/Classes/Persistence/Doctrine/EntityManagerConfiguration.php

Line 229 in d93b6b0

$entityManager->getConnection()->getDatabasePlatform()->registerDoctrineTypeMapping($typeConfiguration['dbType'], $typeName);

This is ironic because if there is no connection - or no entity manager in the first place, the current process cannot have made any changes to the transaction.

Upgrade instructions

Review instructions

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[kitsunet]

Please have a look at the Flow Package.php, which runs the persistAll after controller. There is a condition in there on:

$bootstrap->getObjectManager()->has(Persistence\PersistenceManagerInterface::class)

this is wrong though, not sure if that has ever changed in the object manager or if the condition was wrong since ever, but flipping that to:

$bootstrap->getObjectManager()->hasInstance(Persistence\PersistenceManagerInterface::class)

Should fix this as well?!

[kdambekalns]

Please have a look at the Flow Package.php, which runs the persistAll after controller. There is a condition in there on:

Good catch!

$bootstrap->getObjectManager()->has(Persistence\PersistenceManagerInterface::class)

this is wrong though, not sure if that has ever changed in the object manager

The has() was added with #1163 by @kitsunet 😎

or if the condition was wrong since ever, but flipping that to:

$bootstrap->getObjectManager()->hasInstance(Persistence\PersistenceManagerInterface::class)

That was changed from hasinstance() to has() in #1366 (a PR by @kitsunet) – but in this case it was @kdambekalns who made that particular commit in that PR. 🙈

Whatever made me think hasInstance() is deprecated and has() was the replacement… 🤷‍♂️

[kitsunet]

Thanks for looking that up.

I actually expected it was myself breaking my own code from before 😂. has is part of the Psr ContainerInterface so that might have looked sensible in the "change to PSR" methods change. Sadly there is no ContainerInterface replacement for hasInstance

[mhsdesign]

Should fix this as well?!

indeed. Good catch thank you both for digging into that as well.

Adjusted that and tested it:)