Skip to content

Commit 8cec7c7

Browse files
committed
Fix MongoDB ignore+transaction, PHPStan errors, Postgres quoting, and ACL drift
- Pre-filter known-duplicate docs before passing to adapter, preventing permissions being inserted for existing docs (ACL drift) and fixing MongoDB transaction abort on BulkWriteException - Skip transaction session in Mongo adapter when ignore=true so ordered:false inserts persist even if some fail - Return empty array from Mongo catch block instead of untransformed cloned inputs - Quote all column names in Postgres ON CONFLICT targets consistently - Wrap array_chunk calls with max(1, ...) to satisfy PHPStan int<1,max>
1 parent e85729d commit 8cec7c7

3 files changed

Lines changed: 29 additions & 12 deletions

File tree

src/Database/Adapter/Mongo.php

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,10 +1464,12 @@ public function createDocuments(Document $collection, array $documents, bool $ig
14641464
{
14651465
$name = $this->getNamespace() . '_' . $this->filter($collection->getId());
14661466

1467-
$options = $this->getTransactionOptions();
1468-
14691467
if ($ignore) {
1470-
$options['ordered'] = false;
1468+
// Run outside transaction — MongoDB aborts transactions on any write error,
1469+
// so ordered:false + session would roll back even successfully inserted docs.
1470+
$options = ['ordered' => false];
1471+
} else {
1472+
$options = $this->getTransactionOptions();
14711473
}
14721474

14731475
$records = [];
@@ -1498,7 +1500,10 @@ public function createDocuments(Document $collection, array $documents, bool $ig
14981500
$processed = $this->processException($e);
14991501

15001502
if ($ignore && $processed instanceof DuplicateException) {
1501-
return $documents;
1503+
// Race condition: a doc was inserted between pre-filter and insertMany.
1504+
// With ordered:false outside transaction, non-duplicate inserts persist.
1505+
// Return empty — we cannot determine which docs succeeded without querying.
1506+
return [];
15021507
}
15031508

15041509
throw $processed;

src/Database/Adapter/Postgres.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,7 +1376,7 @@ protected function getInsertSuffix(bool $ignore, string $table): string
13761376
return '';
13771377
}
13781378

1379-
$conflictTarget = $this->sharedTables ? '("_uid", _tenant)' : '("_uid")';
1379+
$conflictTarget = $this->sharedTables ? '("_uid", "_tenant")' : '("_uid")';
13801380

13811381
return "ON CONFLICT {$conflictTarget} DO NOTHING";
13821382
}
@@ -1388,8 +1388,8 @@ protected function getInsertPermissionsSuffix(bool $ignore): string
13881388
}
13891389

13901390
$conflictTarget = $this->sharedTables
1391-
? '(_type, _permission, _document, _tenant)'
1392-
: '(_type, _permission, _document)';
1391+
? '("_type", "_permission", "_document", "_tenant")'
1392+
: '("_type", "_permission", "_document")';
13931393

13941394
return "ON CONFLICT {$conflictTarget} DO NOTHING";
13951395
}

src/Database/Database.php

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5659,15 +5659,15 @@ public function createDocuments(
56595659
// pre-fetch existing document IDs so we skip relationship writes for duplicates
56605660
$preExistingIds = [];
56615661
$tenantPerDocument = $this->adapter->getSharedTables() && $this->adapter->getTenantPerDocument();
5662-
if ($ignore && $this->resolveRelationships) {
5662+
if ($ignore) {
56635663
if ($tenantPerDocument) {
56645664
$idsByTenant = [];
56655665
foreach ($documents as $doc) {
56665666
$idsByTenant[$doc->getTenant()][] = $doc->getId();
56675667
}
56685668
foreach ($idsByTenant as $tenant => $tenantIds) {
56695669
$tenantIds = \array_values(\array_unique($tenantIds));
5670-
foreach (\array_chunk($tenantIds, $this->maxQueryValues) as $idChunk) {
5670+
foreach (\array_chunk($tenantIds, \max(1, $this->maxQueryValues)) as $idChunk) {
56715671
$existing = $this->authorization->skip(fn () => $this->withTenant($tenant, fn () => $this->silent(fn () => $this->find(
56725672
$collection->getId(),
56735673
[
@@ -5686,7 +5686,7 @@ public function createDocuments(
56865686
\array_map(fn (Document $doc) => $doc->getId(), $documents)
56875687
)));
56885688

5689-
foreach (\array_chunk($inputIds, $this->maxQueryValues) as $idChunk) {
5689+
foreach (\array_chunk($inputIds, \max(1, $this->maxQueryValues)) as $idChunk) {
56905690
$existing = $this->authorization->skip(fn () => $this->silent(fn () => $this->find(
56915691
$collection->getId(),
56925692
[
@@ -5755,6 +5755,18 @@ public function createDocuments(
57555755
}
57565756

57575757
foreach (\array_chunk($documents, $batchSize) as $chunk) {
5758+
if ($ignore && !empty($preExistingIds)) {
5759+
$chunk = \array_values(\array_filter($chunk, function (Document $doc) use ($preExistingIds, $tenantPerDocument) {
5760+
$key = $tenantPerDocument
5761+
? $doc->getTenant() . ':' . $doc->getId()
5762+
: $doc->getId();
5763+
return !isset($preExistingIds[$key]);
5764+
}));
5765+
if (empty($chunk)) {
5766+
continue;
5767+
}
5768+
}
5769+
57585770
$batch = $this->withTransaction(function () use ($collection, $chunk, $ignore) {
57595771
return $this->adapter->createDocuments($collection, $chunk, $ignore);
57605772
});
@@ -7190,7 +7202,7 @@ public function upsertDocumentsWithIncrease(
71907202
}
71917203
foreach ($idsByTenant as $tenant => $tenantIds) {
71927204
$tenantIds = \array_values(\array_unique($tenantIds));
7193-
foreach (\array_chunk($tenantIds, $this->maxQueryValues) as $idChunk) {
7205+
foreach (\array_chunk($tenantIds, \max(1, $this->maxQueryValues)) as $idChunk) {
71947206
$fetched = $this->authorization->skip(fn () => $this->withTenant($tenant, fn () => $this->silent(fn () => $this->find(
71957207
$collection->getId(),
71967208
[Query::equal('$id', $idChunk), Query::limit(\count($idChunk))],
@@ -7201,7 +7213,7 @@ public function upsertDocumentsWithIncrease(
72017213
}
72027214
}
72037215
} else {
7204-
foreach (\array_chunk($uniqueIds, $this->maxQueryValues) as $idChunk) {
7216+
foreach (\array_chunk($uniqueIds, \max(1, $this->maxQueryValues)) as $idChunk) {
72057217
$fetched = $this->authorization->skip(fn () => $this->silent(fn () => $this->find(
72067218
$collection->getId(),
72077219
[Query::equal('$id', $idChunk), Query::limit(\count($idChunk))],

0 commit comments

Comments
 (0)