Skip to content

Commit b9727f2

Browse files
abnegateclaude
andcommitted
fix: address review comments and fix test failures
- Wrap deleteCollection in try/catch for NotFoundException in duplicate recovery flow - Remove dead $created condition (always true, flagged by PHPStan) - Skip Sequence validation for $tenant attribute - Use loose comparison for tenant checks to handle type mismatches Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fa3b6ff commit b9727f2

2 files changed

Lines changed: 18 additions & 17 deletions

File tree

src/Database/Database.php

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1790,19 +1790,19 @@ public function createCollection(string $id, array $attributes = [], array $inde
17901790
}
17911791
}
17921792

1793-
$created = false;
1794-
17951793
try {
17961794
$this->adapter->createCollection($id, $attributes, $indexes);
1797-
$created = true;
17981795
} catch (DuplicateException $e) {
17991796
// Metadata check (above) already verified collection is absent
18001797
// from metadata. A DuplicateException from the adapter means the
18011798
// collection exists only in physical schema — an orphan from a prior
18021799
// partial failure. Drop and recreate to ensure schema matches.
1803-
$this->adapter->deleteCollection($id);
1800+
try {
1801+
$this->adapter->deleteCollection($id);
1802+
} catch (NotFoundException) {
1803+
// Already removed by a concurrent reconciler.
1804+
}
18041805
$this->adapter->createCollection($id, $attributes, $indexes);
1805-
$created = true;
18061806
}
18071807

18081808
if ($id === self::METADATA) {
@@ -1812,12 +1812,10 @@ public function createCollection(string $id, array $attributes = [], array $inde
18121812
try {
18131813
$createdCollection = $this->silent(fn () => $this->createDocument(self::METADATA, $collection));
18141814
} catch (\Throwable $e) {
1815-
if ($created) {
1816-
try {
1817-
$this->cleanupCollection($id);
1818-
} catch (\Throwable $e) {
1819-
Console::error("Failed to rollback collection '{$id}': " . $e->getMessage());
1820-
}
1815+
try {
1816+
$this->cleanupCollection($id);
1817+
} catch (\Throwable $e) {
1818+
Console::error("Failed to rollback collection '{$id}': " . $e->getMessage());
18211819
}
18221820
throw new DatabaseException("Failed to create collection metadata for '{$id}': " . $e->getMessage(), previous: $e);
18231821
}
@@ -1859,7 +1857,7 @@ public function updateCollection(string $id, array $permissions, bool $documentS
18591857

18601858
if (
18611859
$this->adapter->getSharedTables()
1862-
&& $collection->getTenant() !== $this->adapter->getTenant()
1860+
&& $collection->getTenant() != $this->adapter->getTenant()
18631861
) {
18641862
throw new NotFoundException('Collection not found');
18651863
}
@@ -1895,7 +1893,7 @@ public function getCollection(string $id): Document
18951893
$id !== self::METADATA
18961894
&& $this->adapter->getSharedTables()
18971895
&& $collection->getTenant() !== null
1898-
&& $collection->getTenant() !== $this->adapter->getTenant()
1896+
&& $collection->getTenant() != $this->adapter->getTenant()
18991897
) {
19001898
return new Document();
19011899
}
@@ -1950,7 +1948,7 @@ public function getSizeOfCollection(string $collection): int
19501948
throw new NotFoundException('Collection not found');
19511949
}
19521950

1953-
if ($this->adapter->getSharedTables() && $collection->getTenant() !== $this->adapter->getTenant()) {
1951+
if ($this->adapter->getSharedTables() && $collection->getTenant() != $this->adapter->getTenant()) {
19541952
throw new NotFoundException('Collection not found');
19551953
}
19561954

@@ -1976,7 +1974,7 @@ public function getSizeOfCollectionOnDisk(string $collection): int
19761974
throw new NotFoundException('Collection not found');
19771975
}
19781976

1979-
if ($this->adapter->getSharedTables() && $collection->getTenant() !== $this->adapter->getTenant()) {
1977+
if ($this->adapter->getSharedTables() && $collection->getTenant() != $this->adapter->getTenant()) {
19801978
throw new NotFoundException('Collection not found');
19811979
}
19821980

@@ -2010,7 +2008,7 @@ public function deleteCollection(string $id): bool
20102008
throw new NotFoundException('Collection not found');
20112009
}
20122010

2013-
if ($this->adapter->getSharedTables() && $collection->getTenant() !== $this->adapter->getTenant()) {
2011+
if ($this->adapter->getSharedTables() && $collection->getTenant() != $this->adapter->getTenant()) {
20142012
throw new NotFoundException('Collection not found');
20152013
}
20162014

@@ -7204,7 +7202,7 @@ public function upsertDocumentsWithIncrease(
72047202
if ($document->getTenant() === null) {
72057203
throw new DatabaseException('Missing tenant. Tenant must be set when tenant per document is enabled.');
72067204
}
7207-
if (!$old->isEmpty() && $old->getTenant() !== $document->getTenant()) {
7205+
if (!$old->isEmpty() && $old->getTenant() != $document->getTenant()) {
72087206
throw new DatabaseException('Tenant cannot be changed.');
72097207
}
72107208
} else {

src/Database/Validator/Structure.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,9 @@ protected function checkForInvalidAttributeValues(array $structure, array $keys)
340340

341341
switch ($type) {
342342
case Database::VAR_ID:
343+
if ($attribute['$id'] === '$tenant') {
344+
break;
345+
}
343346
$validators[] = new Sequence($this->idAttributeType, $attribute['$id'] === '$sequence');
344347
break;
345348

0 commit comments

Comments
 (0)