Skip to content

Commit

Permalink
Don't detach entities in in-process batch edit
Browse files Browse the repository at this point in the history
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.

(omeka#1690)
  • Loading branch information
zerocrates committed Feb 25, 2021
1 parent 442e395 commit 578dd20
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 6 deletions.
29 changes: 23 additions & 6 deletions application/src/Api/Adapter/AbstractEntityAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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 = [
Expand All @@ -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 = [];
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions application/src/Controller/Admin/ItemController.php
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ public function batchEditAction()
$this->api($form)->batchUpdate('items', $resourceIds, $properties, [
'continueOnError' => true,
'collectionAction' => $collectionAction,
'detachEntities' => false,
]);
}

Expand Down
1 change: 1 addition & 0 deletions application/src/Controller/Admin/ItemSetController.php
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ public function batchEditAction()
$this->api($form)->batchUpdate('item_sets', $resourceIds, $properties, [
'continueOnError' => true,
'collectionAction' => $collectionAction,
'detachEntites' => false,
]);
}

Expand Down
1 change: 1 addition & 0 deletions application/src/Controller/Admin/MediaController.php
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ public function batchEditAction()
$this->api($form)->batchUpdate('media', $resourceIds, $properties, [
'continueOnError' => true,
'collectionAction' => $collectionAction,
'detachEntities' => false,
]);
}

Expand Down
1 change: 1 addition & 0 deletions application/src/Controller/Admin/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ public function batchEditAction()
$this->api($form)->batchUpdate('users', $resourceIds, $properties, [
'continueOnError' => true,
'collectionAction' => $collectionAction,
'detachEntities' => false,
]);
}

Expand Down

0 comments on commit 578dd20

Please sign in to comment.