Skip to content

PHPORM-268 Add configuration for scout search indexes #3281

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 19, 2025
Merged

PHPORM-268 Add configuration for scout search indexes #3281

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50560d2
PHPORM-268 Add configuration for scout search indexes
GromNaN Feb 17, 2025
cfbf2cc
Improve exception message
GromNaN Feb 19, 2025
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
3 changes: 2 additions & 1 deletion src/MongoDBServiceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,11 @@ private function registerScoutEngine(): void
$connectionName = $app->get('config')->get('scout.mongodb.connection', 'mongodb');
$connection = $app->get('db')->connection($connectionName);
$softDelete = (bool) $app->get('config')->get('scout.soft_delete', false);
$indexDefinitions = $app->get('config')->get('scout.mongodb.index-definitions', []);

assert($connection instanceof Connection, new InvalidArgumentException(sprintf('The connection "%s" is not a MongoDB connection.', $connectionName)));

return new ScoutEngine($connection->getMongoDB(), $softDelete);
return new ScoutEngine($connection->getMongoDB(), $softDelete, $indexDefinitions);
});

return $engineManager;
Expand Down
12 changes: 8 additions & 4 deletions src/Scout/ScoutEngine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ final class ScoutEngine extends Engine

private const TYPEMAP = ['root' => 'object', 'document' => 'bson', 'array' => 'bson'];

/** @param array<string, array> $indexDefinitions */
public function __construct(
private Database $database,
private bool $softDelete,
private array $indexDefinitions = [],
) {
}

Expand Down Expand Up @@ -435,14 +437,16 @@ public function createIndex($name, array $options = []): void
{
assert(is_string($name), new TypeError(sprintf('Argument #1 ($name) must be of type string, %s given', get_debug_type($name))));

$definition = $this->indexDefinitions[$name] ?? self::DEFAULT_DEFINITION;
if (! isset($definition['mappings'])) {
throw new LogicException(sprintf('The search index definition for collection "scout.mongodb.index-definition.%s" must contain a "mappings" key. Find documentation at https://www.mongodb.com/docs/atlas/atlas-search/create-index/', $name));
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure about adding a link here. Can be useful, maybe update it when the laravel docs is written.

Copy link
Member

Choose a reason for hiding this comment

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

Would InvalidArgumentException be preferable, or is there prior art for using LogicException directly?

https://www.mongodb.com/docs/atlas/atlas-search/create-index/ seems fine if you're in the habit of adding doc links to exception messages.

https://www.mongodb.com/docs/manual/reference/command/createSearchIndexes/#search-index-definition-syntax may be a more direct reference, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the exception class and the message with your link.

}

// Ensure the collection exists before creating the search index
$this->database->createCollection($name);

$collection = $this->database->selectCollection($name);
$collection->createSearchIndex(
self::DEFAULT_DEFINITION,
['name' => self::INDEX_NAME],
);
$collection->createSearchIndex($definition, ['name' => self::INDEX_NAME]);

if ($options['wait'] ?? true) {
$this->wait(function () use ($collection) {
Expand Down
79 changes: 79 additions & 0 deletions tests/Scout/ScoutEngineTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@

namespace MongoDB\Laravel\Tests\Scout;

use ArrayIterator;
use Closure;
use DateTimeImmutable;
use Illuminate\Database\Eloquent\Collection as EloquentCollection;
use Illuminate\Support\Collection as LaravelCollection;
use Illuminate\Support\LazyCollection;
use Laravel\Scout\Builder;
use Laravel\Scout\Jobs\RemoveFromSearch;
use LogicException;
use Mockery as m;
use MongoDB\BSON\Document;
use MongoDB\BSON\UTCDateTime;
use MongoDB\Collection;
use MongoDB\Database;
Expand All @@ -31,6 +34,82 @@ class ScoutEngineTest extends TestCase
{
private const EXPECTED_TYPEMAP = ['root' => 'object', 'document' => 'bson', 'array' => 'bson'];

public function testCreateIndexInvalidDefinition(): void
{
$database = m::mock(Database::class);
$engine = new ScoutEngine($database, false, ['collection_invalid' => ['foo' => 'bar']]);

$this->expectException(LogicException::class);
$this->expectExceptionMessage('The search index definition for collection "scout.mongodb.index-definition.collection_invalid" must contain a "mappings" key');
Copy link
Member

Choose a reason for hiding this comment

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

Does this attempt to match the entire message, or is it only a prefix?

Copy link
Member Author

Choose a reason for hiding this comment

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

str_contains is used by PHPUnit for this.

$engine->createIndex('collection_invalid');
}

public function testCreateIndex(): void
{
$collectionName = 'collection_custom';
$expectedDefinition = [
'mappings' => [
'dynamic' => true,
],
];

$database = m::mock(Database::class);
$collection = m::mock(Collection::class);
$database->shouldReceive('createCollection')
->once()
->with($collectionName);
$database->shouldReceive('selectCollection')
->with($collectionName)
->andReturn($collection);
$collection->shouldReceive('createSearchIndex')
->once()
->with($expectedDefinition, ['name' => 'scout']);
$collection->shouldReceive('listSearchIndexes')
->once()
->with(['name' => 'scout', 'typeMap' => ['root' => 'bson']])
->andReturn(new ArrayIterator([Document::fromPHP(['name' => 'scout', 'status' => 'READY'])]));

$engine = new ScoutEngine($database, false, []);
$engine->createIndex($collectionName);
}

public function testCreateIndexCustomDefinition(): void
{
$collectionName = 'collection_custom';
$expectedDefinition = [
'mappings' => [
[
'analyzer' => 'lucene.standard',
'fields' => [
[
'name' => 'wildcard',
'type' => 'string',
],
],
],
],
];

$database = m::mock(Database::class);
$collection = m::mock(Collection::class);
$database->shouldReceive('createCollection')
->once()
->with($collectionName);
$database->shouldReceive('selectCollection')
->with($collectionName)
->andReturn($collection);
$collection->shouldReceive('createSearchIndex')
->once()
->with($expectedDefinition, ['name' => 'scout']);
$collection->shouldReceive('listSearchIndexes')
->once()
->with(['name' => 'scout', 'typeMap' => ['root' => 'bson']])
->andReturn(new ArrayIterator([Document::fromPHP(['name' => 'scout', 'status' => 'READY'])]));

$engine = new ScoutEngine($database, false, [$collectionName => $expectedDefinition]);
$engine->createIndex($collectionName);
}

/** @param callable(): Builder $builder */
#[DataProvider('provideSearchPipelines')]
public function testSearch(Closure $builder, array $expectedPipeline): void
Expand Down
7 changes: 6 additions & 1 deletion tests/Scout/ScoutIntegrationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use function array_merge;
use function count;
use function env;
use function iterator_to_array;
use function Orchestra\Testbench\artisan;
use function range;
use function sprintf;
Expand All @@ -38,6 +39,9 @@ protected function getEnvironmentSetUp($app): void

$app['config']->set('scout.driver', 'mongodb');
$app['config']->set('scout.prefix', 'prefix_');
$app['config']->set('scout.mongodb.index-definitions', [
'prefix_scout_users' => ['mappings' => ['dynamic' => true, 'fields' => ['bool_field' => ['type' => 'boolean']]]],
Copy link
Member

Choose a reason for hiding this comment

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

Is it sensible to combine dynamic: true with explicit field mappings? I don't recall seeing anything like this in the server manual or Atlas docs.

Copy link
Member Author

@GromNaN GromNaN Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It's allowed. It forces some field types but enable automatic indexation for others. That's useful for me here as I want to ensure the dynamic mapping works with my search query, but also want to test a custom definition.

]);
}

public function setUp(): void
Expand Down Expand Up @@ -103,8 +107,9 @@ public function testItCanCreateTheCollection()

self::assertSame(44, $collection->countDocuments());

$searchIndexes = $collection->listSearchIndexes(['name' => 'scout']);
$searchIndexes = $collection->listSearchIndexes(['name' => 'scout', 'typeMap' => ['root' => 'array', 'document' => 'array', 'array' => 'array']]);
self::assertCount(1, $searchIndexes);
self::assertSame(['mappings' => ['dynamic' => true, 'fields' => ['bool_field' => ['type' => 'boolean']]]], iterator_to_array($searchIndexes)[0]['latestDefinition']);

// Wait for all documents to be indexed asynchronously
$i = 100;
Expand Down
Loading