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

Fix illegal argument exception when use UUIDs as primary keys #63

Merged
merged 12 commits into from
Jun 15, 2023
12 changes: 3 additions & 9 deletions src/Engines/OpenSearchEngine.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function update($models): void
}

return array_merge($searchableData, $model->scoutMetadata(), [
'id' => $model->getScoutKey(),
$model->getScoutKeyName() => $model->getScoutKey(),
]);
})->filter()
->values()
Expand All @@ -60,7 +60,7 @@ public function update($models): void
$data[] = [
'index' => [
'_index' => $model->searchableAs(),
'_id' => $object['id'],
'_id' => $object[$model->getScoutKeyName()],
],
];
$data[] = $object;
Expand Down Expand Up @@ -178,13 +178,7 @@ protected function performSearch(Builder $builder, array $options = []): mixed
$order['column'] => [
'order' => $order['direction'],
],
])->whenEmpty(static fn (): Collection => collect([
[
'id' => [
'order' => 'desc',
],
],
]))->all();
])->all();

$result = $this->client->search([
'index' => $index,
Expand Down
41 changes: 41 additions & 0 deletions tests/Fixtures/SearchableModelHasUuids.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

declare(strict_types=1);

namespace Zing\LaravelScout\OpenSearch\Tests\Fixtures;

use Illuminate\Database\Eloquent\Concerns\HasUuids;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\SoftDeletes;
use Laravel\Scout\Searchable;

/**
* @property string $name
* @property int $is_visible
*
* @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder query()
*/
class SearchableModelHasUuids extends Model
{
use HasUuids;
use Searchable;
use SoftDeletes;

protected $primaryKey = 'uuid';

/**
* @return array{name: string, is_visible: int}
*/
public function toSearchableArray(): array
{
return [
'name' => $this->name,
'is_visible' => $this->is_visible,
];
}

/**
* @var string[]
*/
protected $fillable = ['name', 'is_visible'];
}
41 changes: 23 additions & 18 deletions tests/OpenSearchEngineTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ protected function setUp(): void
public function testUpdateAddsObjectsToIndex(): void
{
$client = m::mock(Client::class);
$model = new SearchableModel([
'id' => 1,
]);
$client->shouldReceive('bulk')
->once()
->with([
Expand All @@ -47,18 +50,16 @@ public function testUpdateAddsObjectsToIndex(): void
'_id' => 1,
],
],
[
array_merge([
'id' => 1,
],
], [
$model->getScoutKeyName() => $model->getScoutKey(),
]),
],
]);

$engine = new OpenSearchEngine($client);
$engine->update(Collection::make([
new SearchableModel([
'id' => 1,
]),
]));
$engine->update(Collection::make([$model]));
}

public function testDeleteRemovesObjectsToIndex(): void
Expand Down Expand Up @@ -217,7 +218,8 @@ public function testSearchSendsCorrectParametersToAlgolia(): void

$engine = new OpenSearchEngine($client);
$builder = new Builder(new SearchableModel(), 'zonda');
$builder->where('foo', 1);
$builder->where('foo', 1)
->orderBy('id', 'desc');
$engine->search($builder);
}

Expand Down Expand Up @@ -268,7 +270,8 @@ public function testSearchSendsCorrectParametersToAlgoliaForWhereInSearch(): voi
$engine = new OpenSearchEngine($client);
$builder = new Builder(new SearchableModel(), 'zonda');
$builder->where('foo', 1)
->whereIn('bar', [1, 2]);
->whereIn('bar', [1, 2])
->orderBy('id', 'desc');
$engine->search($builder);
}

Expand Down Expand Up @@ -319,7 +322,8 @@ public function testSearchSendsCorrectParametersToAlgoliaForEmptyWhereInSearch()
$engine = new OpenSearchEngine($client);
$builder = new Builder(new SearchableModel(), 'zonda');
$builder->where('foo', 1)
->whereIn('bar', []);
->whereIn('bar', [])
->orderBy('id', 'desc');
$engine->search($builder);
}

Expand Down Expand Up @@ -519,6 +523,9 @@ public function testLazyMapMethodRespectsOrder(): void
public function testAModelIsIndexedWithACustomAlgoliaKey(): void
{
$client = m::mock(Client::class);
$model = new CustomKeySearchableModel([
'id' => 1,
]);
$client->shouldReceive('bulk')
->once()
->with([
Expand All @@ -530,18 +537,16 @@ public function testAModelIsIndexedWithACustomAlgoliaKey(): void
'_id' => 'my-opensearch-key.1',
],
],
[
'id' => 'my-opensearch-key.1',
],
array_merge([
'id' => 1,
], [
$model->getScoutKeyName() => $model->getScoutKey(),
]),
],
]);

$engine = new OpenSearchEngine($client);
$engine->update(Collection::make([
new CustomKeySearchableModel([
'id' => 1,
]),
]));
$engine->update(Collection::make([$model]));
}

public function testAModelIsRemovedWithACustomAlgoliaKey(): void
Expand Down
116 changes: 116 additions & 0 deletions tests/ScoutHasUuidsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
<?php

declare(strict_types=1);

namespace Zing\LaravelScout\OpenSearch\Tests;

use Illuminate\Database\Eloquent\Concerns\HasUuids;
use Illuminate\Foundation\Testing\WithFaker;
use Laravel\Scout\Builder;
use Zing\LaravelScout\OpenSearch\Tests\Fixtures\SearchableModelHasUuids;

/**
* @internal
*/
final class ScoutHasUuidsTest extends TestCase
{
use WithFaker;

public static function setUpBeforeClass(): void
{
if (! trait_exists(HasUuids::class)) {
self::markTestSkipped('Support for HasUuids available since 9.0.');
}

parent::setUpBeforeClass();
}

protected function setUp(): void
{
parent::setUp();

$searchableModelHasUuids = new SearchableModelHasUuids();
$searchableModelHasUuids->searchableUsing()
->createIndex($searchableModelHasUuids->searchableAs());
}

protected function tearDown(): void
{
$searchableModelHasUuids = new SearchableModelHasUuids();
$searchableModelHasUuids->searchableUsing()
->deleteIndex($searchableModelHasUuids->searchableAs());

parent::tearDown();
}

public function testSearch(): void
{
SearchableModelHasUuids::query()->create([
'name' => 'test search 1',
]);
SearchableModelHasUuids::query()->create([
'name' => 'test search 2',
]);
SearchableModelHasUuids::query()->create([
'name' => 'test search 3',
]);
SearchableModelHasUuids::query()->create([
'name' => 'test search 4',
]);
SearchableModelHasUuids::query()->create([
'name' => 'test search 5',
]);
SearchableModelHasUuids::query()->create([
'name' => 'test search 6',
]);
SearchableModelHasUuids::query()->create([
'name' => 'not matched',
]);
sleep(1);
self::assertCount(6, SearchableModelHasUuids::search('test')->get());
SearchableModelHasUuids::query()->firstOrFail()->delete();
sleep(1);
self::assertCount(5, SearchableModelHasUuids::search('test')->get());
self::assertCount(1, SearchableModelHasUuids::search('test')->paginate(2, 'page', 3)->items());
if (method_exists(Builder::class, 'cursor')) {
self::assertCount(5, SearchableModelHasUuids::search('test')->cursor());
}

self::assertCount(5, SearchableModelHasUuids::search('test')->keys());
SearchableModelHasUuids::removeAllFromSearch();
sleep(1);
self::assertCount(0, SearchableModelHasUuids::search('test')->get());
self::assertCount(0, SearchableModelHasUuids::search('test')->paginate(2, 'page', 3)->items());
if (method_exists(Builder::class, 'cursor')) {
self::assertCount(0, SearchableModelHasUuids::search('test')->cursor());
}

self::assertCount(0, SearchableModelHasUuids::search('test')->keys());
}

public function testWhere(): void
{
SearchableModelHasUuids::query()->create([
'name' => 'test',
'is_visible' => 1,
]);
SearchableModelHasUuids::query()->create([
'name' => 'test',
'is_visible' => 1,
]);
SearchableModelHasUuids::query()->create([
'name' => 'test',
'is_visible' => 0,
]);
SearchableModelHasUuids::query()->create([
'name' => 'nothing',
]);
sleep(1);
self::assertCount(3, SearchableModelHasUuids::search('test')->get());
self::assertCount(2, SearchableModelHasUuids::search('test')->where('is_visible', 1)->get());
if (method_exists(Builder::class, 'whereIn')) {
self::assertCount(3, SearchableModelHasUuids::search('test')->whereIn('is_visible', [0, 1])->get());
self::assertCount(0, SearchableModelHasUuids::search('test')->whereIn('is_visible', [])->get());
}
}
}
2 changes: 1 addition & 1 deletion tests/ScoutTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public function testOrderBy(): void
'name' => 'test search 3',
]);
sleep(1);
self::assertSame(3, SearchableModel::search('test')->first()->getKey());
self::assertSame(3, SearchableModel::search('test')->orderBy('id', 'desc')->first()->getKey());
self::assertSame(1, SearchableModel::search('test')->orderBy('id')->first()->getKey());
self::assertSame(3, SearchableModel::search('test')->orderBy('id', 'desc')->first()->getKey());
}
Expand Down
12 changes: 12 additions & 0 deletions tests/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,17 @@ static function (Blueprint $table): void {
$table->softDeletes();
}
);
DB::connection()->getSchemaBuilder()->create(
'searchable_model_has_uuids',
static function (Blueprint $table): void {
$table->uuid('uuid');
$table->string('name')
->default('');
$table->boolean('is_visible')
->default(true);
$table->timestamps();
$table->softDeletes();
}
);
}
}