neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
260 stars 221 forks source link

Asset replacing broken #2884

Closed daniellienert closed 4 years ago

daniellienert commented 4 years ago

Description

Replacing an asset using the media manager results in "Could not replace asset"

Steps to Reproduce

Reproducable also with Neos dev collection and the demo site:

  1. Select an image in the asset manager
  2. Click on "Replace Asset Resource"
  3. Click on "Choose new File"
  4. Click on "Replace Asset Resource"

Expected behavior

Resource is replaced.

Actual behavior

Error is shown "Could not replace asset".

The actual error is catched and hidden by https://github.com/neos/neos-development-collection/blob/2cfe546beaa56d6040e3463b99cc12de6772d7a1/Neos.Media.Browser/Classes/Controller/AssetController.php#L638

If the catch is removed, the actual doctrine error shows up:

An exception occurred while executing 'DELETE FROM neos_flow_resourcemanagement_persistentresource WHERE persistence_object_identifier = ?' with params ["690714af-0a73-459c-ac4b-da8e1b0d9ca5"]:

SQLSTATE[23000]: Integrity constraint violation: 1451 Cannot delete or update a parent row: a foreign key constraint fails (`production`.`neos_media_domain_model_asset`, CONSTRAINT `FK_B8306B8EBC91F416` FOREIGN KEY (`resource`) REFERENCES `neos_flow_resourcemanagement_persistentresource` (`persistence_object_identi)

It seems that during the replace process, a new image variant is generated and persisted which references the to be replaced resource. When doctrine tries to remove the orphaned resource after it is replaced, the reference denies that.

While investigating what has been changed I stumbled over the big refactoring in https://github.com/neos/neos-development-collection/pull/2751 - but not sure yet it is related.

Affected Versions

Neos: 5.1 / master

Flow: 6.1 / master

dlubitz commented 4 years ago

I can confirm this bug in Neos 5.1.4.

Exception in line 779 of /.../releases/20200123143102/Packages/Libraries/doctrine/orm/lib/Doctrine/ORM/EntityManager.php: The EntityManager is closed.

19 Doctrine\ORM\ORMException::entityManagerClosed()
18 Doctrine\ORM\EntityManager::errorIfClosed()
17 Doctrine\ORM\EntityManager::remove(Neos\Flow\ResourceManagement\PersistentResource)
16 Neos\Flow\Persistence\Doctrine\PersistenceManager_Original::remove(Neos\Flow\ResourceManagement\PersistentResource)
15 Neos\Flow\Persistence\Repository::remove(Neos\Flow\ResourceManagement\PersistentResource)
14 Neos\Flow\ResourceManagement\ResourceRepository_Original::remove(Neos\Flow\ResourceManagement\PersistentResource)
13 Neos\Flow\ResourceManagement\ResourceManager_Original::deleteResource(Neos\Flow\ResourceManagement\PersistentResource, false)
12 Neos\Flow\ResourceManagement\ResourceManager_Original::shutdownObject()
11 Neos\Flow\ObjectManagement\ObjectManager::callShutdownMethods(SplObjectStorage)
10 Neos\Flow\ObjectManagement\ObjectManager::Neos\Flow\ObjectManagement\{closure}()
9 Closure::__invoke()
8 Neos\Flow\Security\Context_Original::withoutAuthorizationChecks(Closure)
7 Neos\Flow\ObjectManagement\ObjectManager::shutdown("Runtime", "Neos\Flow\Core\Bootstrap::bootstrapShuttingDown")
6 call_user_func_array(array|2|, array|2|)
5 Neos\Flow\SignalSlot\Dispatcher::dispatch("Neos\Flow\Core\Bootstrap", "bootstrapShuttingDown", array|1|)
4 Neos\Flow\Core\Bootstrap::emitBootstrapShuttingDown("Runtime")
3 Neos\Flow\Core\Bootstrap::shutdown("Runtime")
2 Neos\Flow\Http\RequestHandler::handleRequest()
1 Neos\Flow\Core\Bootstrap::run()
daniellienert commented 4 years ago

@kdambekalns - I found the PR #2751 which changed a lot in that area. Do you have a clue where to search for the issue?

kitsunet commented 4 years ago

looks to me like the the ORM was already shutdown when this shutdown happens? 🤔

daniellienert commented 4 years ago

@kitsunet before that ORM shutdown, a ForeignKeyConstraintViolationException is thrown:

Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException
An exception occurred while executing 'DELETE FROM neos_flow_resourcemanagement_persistentresource WHERE persistence_object_identifier = ?' with params ["690714af-0a73-459c-ac4b-da8e1b0d9ca5"]:

SQLSTATE[23000]: Integrity constraint violation: 1451 Cannot delete or update a parent row: a foreign key constraint fails (`production`.`neos_media_domain_model_asset`, CONSTRAINT `FK_B8306B8EBC91F416` FOREIGN KEY (`resource`) REFERENCES `neos_flow_resourcemanagement_persistentresource` (`persistence_object_identi)
kdambekalns commented 4 years ago

It seems a lot is going on in that code area, I'll try to dig into that the next days…

kdambekalns commented 4 years ago

With the current 5.1 I cannot reproduce this using the demo site… 😳

dlubitz commented 4 years ago

I had also issues to reproduce it in my staging ... but it seem to be related to the option "Generate redirects from original file url to the new url". If I uncheck this, I can replace the images. If I check this, I get this error.

kdambekalns commented 4 years ago

Ah, I'll try that. That's an important difference, then. Because without that, everything works fine.

daniellienert commented 4 years ago

For me, the problem occurs every time I tried to replace an .png with a jpg or vice-versa.

kdambekalns commented 4 years ago

@daniellienert And that is with a released 5.1 or the branch tip?

kdambekalns commented 4 years ago

@daniellienert

every time I tried to replace an .png with a png or vice-versa.

Hm… did you want to mention two different formats?

daniellienert commented 4 years ago

@kdambekalns 😂 - yeah png and jpg

daniellienert commented 4 years ago

Screencast showing the error. Vanilla dev collection on Neos 6.1 / Flow 6.1

ezgif com-video-to-gif

dlubitz commented 4 years ago

Screencast showing the error. Vanilla dev collection on Neos 6.1 / Flow 6.1

But the option "Generate redirects from original file url to the new" is enabled. Try to replace with unticked option @daniellienert .

daniellienert commented 4 years ago

@dlubitz Works without the "Generate Redirects" option. But the option is default and I like to have it :)

dlubitz commented 4 years ago

Sure, just to to nail down the real issue.

kdambekalns commented 4 years ago

@daniellienert Interesting…

The latter makes me think it must be something related to redirects, thus adding/keeping the add() and update() would rather be a workaround…

kdambekalns commented 4 years ago

I give up. I still don't fully understand why this only happens when redirects are enabled, but anyway. I am at a point where I can see what (probably) happens (see https://github.com/neos/neos-development-collection/pull/2892/commits/d85358c4f08024ade50b434ad934ce12e7b33829) and I hope this fixes it for you, too.

daniellienert commented 4 years ago

Thanks @kdambekalns. Seems to be fixed now.