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) (cherry picked from commit 578dd20)
omeka · Mar 26, 2021 · a3708a6 · a3708a6
1 parent 9d2db42
commit a3708a6
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -385,6 +385,7 @@ public function batchEditAction()
                     $this->api($form)->batchUpdate('users', $resourceIds, $properties, [
                         'continueOnError' => true,
                         'collectionAction' => $collectionAction,
+                        'detachEntities' => false,
                     ]);
                 }