Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gracefully handle search index exceptions if possible #2671

Merged
merged 2 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 40 additions & 3 deletions lib/Doctrine/ODM/MongoDB/SchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Doctrine\ODM\MongoDB\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ODM\MongoDB\Repository\ViewRepository;
use InvalidArgumentException;
use MongoDB\Driver\Exception\CommandException;
use MongoDB\Driver\Exception\RuntimeException;
use MongoDB\Driver\Exception\ServerException;
use MongoDB\Driver\WriteConcern;
Expand All @@ -32,6 +33,7 @@
use function iterator_to_array;
use function ksort;
use function sprintf;
use function str_contains;

/**
* @psalm-import-type IndexMapping from ClassMetadata
Expand All @@ -44,6 +46,7 @@ final class SchemaManager
private const GRIDFS_CHUNKS_COLLECTION_INDEX = ['filename' => 1, 'uploadDate' => 1];

private const CODE_SHARDING_ALREADY_INITIALIZED = 23;
private const CODE_COMMAND_NOT_SUPPORTED = 115;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there are no consts anywhere in drivers? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope :)


private const ALLOWED_MISSING_INDEX_OPTIONS = [
'background',
Expand Down Expand Up @@ -408,8 +411,19 @@ public function updateDocumentSearchIndexes(string $documentName): void
$searchIndexes = $class->getSearchIndexes();
$collection = $this->dm->getDocumentCollection($class->name);

$definedNames = array_column($searchIndexes, 'name');
$existingNames = array_column(iterator_to_array($collection->listSearchIndexes()), 'name');
$definedNames = array_column($searchIndexes, 'name');
try {
$existingNames = array_column(iterator_to_array($collection->listSearchIndexes()), 'name');
} catch (CommandException $e) {
/* If $listSearchIndexes doesn't exist, only throw if search indexes have been defined.
* If no search indexes are defined and the server doesn't support search indexes, there's
* nothing for us to do here and we can safely return */
if ($definedNames === [] && $this->isSearchIndexCommandException($e)) {
return;
}

throw $e;
}

foreach (array_diff($existingNames, $definedNames) as $name) {
$collection->dropSearchIndex($name);
Expand Down Expand Up @@ -450,7 +464,18 @@ public function deleteDocumentSearchIndexes(string $documentName): void

$collection = $this->dm->getDocumentCollection($class->name);

foreach ($collection->listSearchIndexes() as $searchIndex) {
try {
$searchIndexes = $collection->listSearchIndexes();
} catch (CommandException $e) {
// If the server does not support search indexes, there are no indexes to remove in any case
if ($this->isSearchIndexCommandException($e)) {
return;
}

throw $e;
}

foreach ($searchIndexes as $searchIndex) {
$collection->dropSearchIndex($searchIndex['name']);
}
}
Expand Down Expand Up @@ -1029,4 +1054,16 @@ private function getWriteOptions(?int $maxTimeMs = null, ?WriteConcern $writeCon

return $options;
}

private function isSearchIndexCommandException(CommandException $e): bool
{
// MongoDB 6.0.7+ and 7.0+: "Search indexes are only available on Atlas"
if ($e->getCode() === self::CODE_COMMAND_NOT_SUPPORTED && str_contains($e->getMessage(), 'Search index')) {
return true;
}

// Older server versions don't support $listSearchIndexes
// We don't check for an error code here as the code is not documented and we can't rely on it
return str_contains($e->getMessage(), 'Unrecognized pipeline stage name: \'$listSearchIndexes\'');
}
}
107 changes: 107 additions & 0 deletions tests/Doctrine/ODM/MongoDB/Tests/SchemaManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use MongoDB\Client;
use MongoDB\Collection;
use MongoDB\Database;
use MongoDB\Driver\Exception\CommandException;
use MongoDB\Driver\WriteConcern;
use MongoDB\GridFS\Bucket;
use MongoDB\Model\CollectionInfo;
Expand Down Expand Up @@ -420,6 +421,27 @@ public function testCreateDocumentSearchIndexes(): void
$this->schemaManager->createDocumentSearchIndexes(CmsArticle::class);
}

public function testCreateDocumentSearchIndexesNotSupported(): void
{
$exception = $this->createSearchIndexCommandException();

$cmsArticleCollectionName = $this->dm->getClassMetadata(CmsArticle::class)->getCollection();
foreach ($this->documentCollections as $collectionName => $collection) {
if ($collectionName === $cmsArticleCollectionName) {
$collection
->expects($this->once())
->method('createSearchIndexes')
->with($this->anything())
->willThrowException($exception);
} else {
$collection->expects($this->never())->method('createSearchIndexes');
}
}

$this->expectExceptionObject($exception);
$this->schemaManager->createDocumentSearchIndexes(CmsArticle::class);
}

public function testUpdateDocumentSearchIndexes(): void
{
$collectionName = $this->dm->getClassMetadata(CmsArticle::class)->getCollection();
Expand All @@ -443,6 +465,66 @@ public function testUpdateDocumentSearchIndexes(): void
$this->schemaManager->updateDocumentSearchIndexes(CmsArticle::class);
}

public function testUpdateDocumentSearchIndexesNotSupportedForClassWithoutSearchIndexes(): void
{
// Class has no search indexes, so if the server doesn't support them we assume everything is fine
$collectionName = $this->dm->getClassMetadata(CmsProduct::class)->getCollection();
$collection = $this->documentCollections[$collectionName];
$collection
->expects($this->once())
->method('listSearchIndexes')
->willThrowException($this->createSearchIndexCommandException());
$collection
->expects($this->never())
->method('dropSearchIndex');
$collection
->expects($this->never())
->method('updateSearchIndex');

$this->schemaManager->updateDocumentSearchIndexes(CmsProduct::class);
}

public function testUpdateDocumentSearchIndexesNotSupportedForClassWithoutSearchIndexesOnOlderServers(): void
{
// Class has no search indexes, so if the server doesn't support them we assume everything is fine
$collectionName = $this->dm->getClassMetadata(CmsProduct::class)->getCollection();
$collection = $this->documentCollections[$collectionName];
$collection
->expects($this->once())
->method('listSearchIndexes')
->willThrowException($this->createSearchIndexCommandExceptionForOlderServers());
$collection
->expects($this->never())
->method('dropSearchIndex');
$collection
->expects($this->never())
->method('updateSearchIndex');

$this->schemaManager->updateDocumentSearchIndexes(CmsProduct::class);
}

public function testUpdateDocumentSearchIndexesNotSupportedForClassWithSearchIndexes(): void
{
$exception = $this->createSearchIndexCommandException();

// This class has search indexes, so we do expect an exception
$collectionName = $this->dm->getClassMetadata(CmsArticle::class)->getCollection();
$collection = $this->documentCollections[$collectionName];
$collection
->expects($this->once())
->method('listSearchIndexes')
->willThrowException($exception);
$collection
->expects($this->never())
->method('dropSearchIndex');
$collection
->expects($this->never())
->method('updateSearchIndex');

$this->expectExceptionObject($exception);
$this->schemaManager->updateDocumentSearchIndexes(CmsArticle::class);
}

public function testDeleteDocumentSearchIndexes(): void
{
$collectionName = $this->dm->getClassMetadata(CmsArticle::class)->getCollection();
Expand All @@ -459,6 +541,21 @@ public function testDeleteDocumentSearchIndexes(): void
$this->schemaManager->deleteDocumentSearchIndexes(CmsArticle::class);
}

public function testDeleteDocumentSearchIndexesNotSupported(): void
{
$collectionName = $this->dm->getClassMetadata(CmsArticle::class)->getCollection();
$collection = $this->documentCollections[$collectionName];
$collection
->expects($this->once())
->method('listSearchIndexes')
->willThrowException($this->createSearchIndexCommandException());
$collection
->expects($this->never())
->method('dropSearchIndex');

$this->schemaManager->deleteDocumentSearchIndexes(CmsArticle::class);
}

public function testUpdateValidators(): void
{
$dbCommands = [];
Expand Down Expand Up @@ -1239,4 +1336,14 @@ private function writeOptions(array $expectedWriteOptions): Constraint
return true;
});
}

private function createSearchIndexCommandException(): CommandException
{
return new CommandException('PlanExecutor error during aggregation :: caused by :: Search index commands are only supported with Atlas.', 115);
}

private function createSearchIndexCommandExceptionForOlderServers(): CommandException
{
return new CommandException('Unrecognized pipeline stage name: \'$listSearchIndexes\'', 40234);
}
}
Loading