Skip to content

Commit

Permalink
Fix illegal argument exception when use UUIDs as primary keys (#63)
Browse files Browse the repository at this point in the history 
* Reproduce issue #62

* Rector Rectify

* Skip ScoutHasUuidsTest when class HasUuids not exists

* Skip ScoutHasUuidsTest when class HasUuids not exists

* Rector Rectify

* Remove default sort option

* Rector Rectify

* Remove default sort option

* Rector Rectify

* Fix phpdoc

---------

Co-authored-by: zingimmick <zingimmick@users.noreply.github.com>
  • Loading branch information
@zingimmick
zingimmick and zingimmick authored Jun 15, 2023
1 parent f70b6c5 commit c1833d3
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 28 deletions.
12 changes: 3 additions & 9 deletions src/Engines/OpenSearchEngine.php
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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();
}
);
}
}

0 comments on commit c1833d3

Please sign in to comment.