From 578dd201f6528d012fa915268d28ac2362c7a9a6 Mon Sep 17 00:00:00 2001 From: John Flatness Date: Thu, 25 Feb 2021 14:48:03 -0500 Subject: [PATCH] Don't detach entities in in-process batch edit The detach process used in batch edit/create is pretty brittle, especially in "synchronous" or in-process contexts where many more entities are loaded before the batch edit begins, so the results are less predictable. Typical "bad" results are Doctrine errors about duplicates or encountering "new" un-persisted entities through a relationship. The detach also serves very little purpose in this context, as it's only there to improve performance _between_ multiple batches, and the in-process edits only do a single batch. This commit adds a new request option for batch edits and creates, "detachEntity", defaulted to true, and has the controllers for the in-process batch updates set it to false. (#1690) --- .../src/Api/Adapter/AbstractEntityAdapter.php | 29 +++++++++++++++---- .../src/Controller/Admin/ItemController.php | 1 + .../Controller/Admin/ItemSetController.php | 1 + .../src/Controller/Admin/MediaController.php | 1 + .../src/Controller/Admin/UserController.php | 1 + 5 files changed, 27 insertions(+), 6 deletions(-) diff --git a/application/src/Api/Adapter/AbstractEntityAdapter.php b/application/src/Api/Adapter/AbstractEntityAdapter.php index 6941d56be9..439b398fee 100644 --- a/application/src/Api/Adapter/AbstractEntityAdapter.php +++ b/application/src/Api/Adapter/AbstractEntityAdapter.php @@ -349,7 +349,11 @@ public function batchCreate(Request $request) $apiManager = $this->getServiceLocator()->get('Omeka\ApiManager'); $logger = $this->getServiceLocator()->get('Omeka\Logger'); - $originalIdentityMap = $this->getEntityManager()->getUnitOfWork()->getIdentityMap(); + $detachEntities = $request->getOption('detachEntities', true); + + if ($detachEntities) { + $originalIdentityMap = $this->getEntityManager()->getUnitOfWork()->getIdentityMap(); + } $subresponses = []; $subrequestOptions = [ @@ -368,7 +372,9 @@ public function batchCreate(Request $request) continue; } // Detatch previously persisted entities before re-throwing. - $this->detachAllNewEntities($originalIdentityMap); + if ($detachEntities) { + $this->detachAllNewEntities($originalIdentityMap); + } throw $e; } $subresponses[$key] = $subresponse; @@ -385,7 +391,9 @@ public function batchCreate(Request $request) $entities[$key] = $entity; } - $this->detachAllNewEntities($originalIdentityMap); + if ($detachEntities) { + $this->detachAllNewEntities($originalIdentityMap); + } $request->setOption('responseContent', 'reference'); return new Response($entities); @@ -420,7 +428,11 @@ public function batchUpdate(Request $request) $apiManager = $this->getServiceLocator()->get('Omeka\ApiManager'); $logger = $this->getServiceLocator()->get('Omeka\Logger'); - $originalIdentityMap = $this->getEntityManager()->getUnitOfWork()->getIdentityMap(); + $detachEntities = $request->getOption('detachEntities', true); + + if ($detachEntities) { + $originalIdentityMap = $this->getEntityManager()->getUnitOfWork()->getIdentityMap(); + } $subresponses = []; $subrequestOptions = [ @@ -441,11 +453,14 @@ public function batchUpdate(Request $request) continue; } // Detatch managed entities before re-throwing. - $this->detachAllNewEntities($originalIdentityMap); + if ($detachEntities) { + $this->detachAllNewEntities($originalIdentityMap); + } throw $e; } $subresponses[$key] = $subresponse; } + $this->getEntityManager()->flush(); $entities = []; @@ -457,7 +472,9 @@ public function batchUpdate(Request $request) $entity = $subresponse->getContent(); $entities[$key] = $entity; } - $this->detachAllNewEntities($originalIdentityMap); + if ($detachEntities) { + $this->detachAllNewEntities($originalIdentityMap); + } $request->setOption('responseContent', 'reference'); return new Response($entities); diff --git a/application/src/Controller/Admin/ItemController.php b/application/src/Controller/Admin/ItemController.php index 19cfec8932..5311979665 100644 --- a/application/src/Controller/Admin/ItemController.php +++ b/application/src/Controller/Admin/ItemController.php @@ -288,6 +288,7 @@ public function batchEditAction() $this->api($form)->batchUpdate('items', $resourceIds, $properties, [ 'continueOnError' => true, 'collectionAction' => $collectionAction, + 'detachEntities' => false, ]); } diff --git a/application/src/Controller/Admin/ItemSetController.php b/application/src/Controller/Admin/ItemSetController.php index 9918b451a4..0a0b30a494 100644 --- a/application/src/Controller/Admin/ItemSetController.php +++ b/application/src/Controller/Admin/ItemSetController.php @@ -258,6 +258,7 @@ public function batchEditAction() $this->api($form)->batchUpdate('item_sets', $resourceIds, $properties, [ 'continueOnError' => true, 'collectionAction' => $collectionAction, + 'detachEntites' => false, ]); } diff --git a/application/src/Controller/Admin/MediaController.php b/application/src/Controller/Admin/MediaController.php index 6fa44eb040..56fc0e2c96 100644 --- a/application/src/Controller/Admin/MediaController.php +++ b/application/src/Controller/Admin/MediaController.php @@ -226,6 +226,7 @@ public function batchEditAction() $this->api($form)->batchUpdate('media', $resourceIds, $properties, [ 'continueOnError' => true, 'collectionAction' => $collectionAction, + 'detachEntities' => false, ]); } diff --git a/application/src/Controller/Admin/UserController.php b/application/src/Controller/Admin/UserController.php index 72cac5c934..abe4273dcf 100644 --- a/application/src/Controller/Admin/UserController.php +++ b/application/src/Controller/Admin/UserController.php @@ -385,6 +385,7 @@ public function batchEditAction() $this->api($form)->batchUpdate('users', $resourceIds, $properties, [ 'continueOnError' => true, 'collectionAction' => $collectionAction, + 'detachEntities' => false, ]); }