Skip to content

Commit 14fc90b

Browse files
committed
cursor() returns a LazyCollection, and survives a fresh connection
Both Query\Builder::cursor() and Eloquent\Builder::cursor() were generator functions handing back a bare Generator, where Laravel's own builders return a LazyCollection. Anything chaining off the framework contract broke, most visibly cursor()->chunk(), which fataled with "Call to undefined method Generator::chunk()". getRawResponse() also dereferenced a null raw response. Only select, insert and aggregate processing assign one and a scroll runs none of them, so a cursor that was the first query on its connection died on asArray(). Every existing cursor test inserted first, which is why nothing caught it. Ported from pdphilip/opensearch v3.2.0. The scroll rewrite from that release is not needed here: the Elasticsearch client already sends scroll_id in the request body for both scroll and clearScroll, so the OpenSearch 3.x "Cannot parse scroll id" failure has no equivalent. Five tests cover it, each verified to fail without the fix: chunking off a cursor, a cursor on a connection that has not run a query, laziness (three hydrations out of ten rows, so an eager collect() fails loudly), and re-iteration. The existing "returns lazy collection via cursor" test was asserting Generator::class, which is now true to its name.
1 parent ebaeadd commit 14fc90b

5 files changed

Lines changed: 115 additions & 16 deletions

File tree

src/Eloquent/Builder.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
use Illuminate\Pagination\CursorPaginator;
1414
use Illuminate\Pagination\Paginator;
1515
use Illuminate\Support\Collection;
16-
use Iterator;
16+
use Illuminate\Support\LazyCollection;
1717
use PDPhilip\Elasticsearch\Data\MetaDTO;
1818
use PDPhilip\Elasticsearch\Exceptions\BuilderException;
1919
use PDPhilip\Elasticsearch\Exceptions\DynamicIndexException;
@@ -378,15 +378,17 @@ protected function searchAfterPaginator($items, $perPage, $cursor, $options)
378378
}
379379

380380
/**
381-
* Get a generator for the given query.
381+
* Get a lazy collection for the given query.
382382
*
383-
* @return Iterator
383+
* @return LazyCollection
384384
*/
385385
public function cursor($scrollTimeout = '30s')
386386
{
387-
foreach ($this->applyScopes()->query->cursor($scrollTimeout) as $record) {
388-
yield $this->model->newFromBuilder($record);
389-
}
387+
return new LazyCollection(function () use ($scrollTimeout) {
388+
foreach ($this->applyScopes()->query->cursor($scrollTimeout) as $record) {
389+
yield $this->model->newFromBuilder($record);
390+
}
391+
});
390392
}
391393

392394
/**

src/Query/Builder.php

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
use DateTimeInterface;
99
use Elastic\Elasticsearch\Response\Elasticsearch;
1010
use Exception;
11-
use Generator;
1211
use Illuminate\Contracts\Support\Arrayable;
1312
use Illuminate\Database\Query\Builder as BaseBuilder;
1413
use Illuminate\Database\Query\Expression;
1514
use Illuminate\Support\Arr;
15+
use Illuminate\Support\LazyCollection;
1616
use Illuminate\Support\Str;
1717
use InvalidArgumentException;
1818
use PDPhilip\Elasticsearch\Connection;
@@ -676,19 +676,21 @@ protected function runPaginationCountQuery($columns = ['_id'])
676676
}
677677

678678
/**
679-
* Get a generator for the given query.
679+
* Get a lazy collection for the given query.
680680
*
681-
* @return Generator
681+
* @return LazyCollection
682682
*/
683683
public function cursor($scrollTimeout = '30s')
684684
{
685-
if (is_null($this->columns)) {
686-
$this->columns = ['*'];
687-
}
685+
return new LazyCollection(function () {
686+
if (is_null($this->columns)) {
687+
$this->columns = ['*'];
688+
}
688689

689-
foreach ($this->connection->cursor($this->toCompiledQuery()) as $document) {
690-
yield $this->processor->documentFromResult($this, $document);
691-
}
690+
foreach ($this->connection->cursor($this->toCompiledQuery()) as $document) {
691+
yield $this->processor->documentFromResult($this, $document);
692+
}
693+
});
692694
}
693695

694696
/**

src/Query/Processor/Processor.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,17 @@ public function getRawAggregationResults(): array
4444

4545
/**
4646
* Get the raw Elasticsearch response as an array
47+
*
48+
* Null is legitimate: only select, insert and aggregate processing assign
49+
* a raw response, and the scroll behind cursor() runs none of them. Per-hit
50+
* meta reaches MetaDTO through documentFromResult's extras either way.
4751
*/
4852
public function getRawResponse(): array
4953
{
54+
if ($this->rawResponse === null) {
55+
return [];
56+
}
57+
5058
return is_array($this->rawResponse) ? $this->rawResponse : $this->rawResponse->asArray();
5159
}
5260

tests/ModelTest.php

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use Carbon\Carbon;
66
use Illuminate\Database\Eloquent\Collection as EloquentCollection;
77
use Illuminate\Database\Eloquent\ModelNotFoundException;
8+
use Illuminate\Support\Facades\DB;
9+
use Illuminate\Support\LazyCollection;
810
use Illuminate\Support\Str;
911
use PDPhilip\Elasticsearch\Connection;
1012
use PDPhilip\Elasticsearch\Data\ModelMeta;
@@ -578,6 +580,75 @@
578580
expect($names)->toHaveCount(15000);
579581
});
580582

583+
it('returns a chunkable lazy collection from cursor', function () {
584+
User::insert([
585+
['name' => 'User 1'],
586+
['name' => 'User 2'],
587+
['name' => 'User 3'],
588+
]);
589+
590+
$cursor = User::query()->orderBy('name.keyword')->cursor();
591+
592+
expect($cursor)->toBeInstanceOf(LazyCollection::class);
593+
594+
$chunkSizes = $cursor->chunk(2)->map(fn ($chunk) => $chunk->count())->all();
595+
expect($chunkSizes)->toBe([2, 1]);
596+
});
597+
598+
it('cursors on a connection that has not run a query yet', function () {
599+
User::insert([
600+
['name' => 'User 1'],
601+
['name' => 'User 2'],
602+
['name' => 'User 3'],
603+
]);
604+
605+
// A scroll populates no raw response, so a cursor that is the first query
606+
// on its connection has no earlier select or insert to inherit one from.
607+
DB::purge('elasticsearch');
608+
609+
$names = User::query()->orderBy('name.keyword')->cursor()->pluck('name')->all();
610+
611+
expect($names)->toBe(['User 1', 'User 2', 'User 3']);
612+
});
613+
614+
it('keeps the cursor lazy, hydrating only what is consumed', function () {
615+
User::insert(array_map(fn ($i) => ['name' => "User {$i}"], range(1, 10)));
616+
617+
$hydrated = 0;
618+
User::retrieved(function () use (&$hydrated) {
619+
$hydrated++;
620+
});
621+
622+
$cursor = User::query()->orderBy('name.keyword')->cursor();
623+
624+
// Building the collection must not touch the cluster.
625+
expect($hydrated)->toBe(0);
626+
627+
$taken = $cursor->take(3)->all();
628+
629+
// Three, not ten - an eagerly materialised collection would fail here while
630+
// every other assertion about cursor() carried on passing.
631+
expect($taken)->toHaveCount(3)
632+
->and($hydrated)->toBe(3);
633+
});
634+
635+
it('re-runs the scroll when the cursor is iterated twice', function () {
636+
User::insert([
637+
['name' => 'User 1'],
638+
['name' => 'User 2'],
639+
['name' => 'User 3'],
640+
]);
641+
642+
$cursor = User::query()->orderBy('name.keyword')->cursor();
643+
644+
// A bare generator throws "Cannot rewind a generator" on the second pass.
645+
$first = $cursor->pluck('name')->all();
646+
$second = $cursor->pluck('name')->all();
647+
648+
expect($first)->toBe(['User 1', 'User 2', 'User 3'])
649+
->and($second)->toBe($first);
650+
});
651+
581652
it('tests truncate model', function () {
582653
User::create(['name' => 'John Doe']);
583654

tests/QueryBuilderTest.php

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Carbon\Carbon;
66
use Illuminate\Support\Facades\Date;
7+
use Illuminate\Support\LazyCollection;
78
use PDPhilip\Elasticsearch\Eloquent\Builder;
89
use PDPhilip\Elasticsearch\Tests\Models\Item;
910
use PDPhilip\Elasticsearch\Tests\Models\User;
@@ -596,12 +597,27 @@
596597

597598
$results = Item::orderBy('name.keyword', 'asc')->cursor();
598599

599-
expect($results)->toBeInstanceOf(Generator::class);
600+
expect($results)->toBeInstanceOf(LazyCollection::class);
600601
foreach ($results as $i => $result) {
601602
expect($result->name)->toBe($data[$i]['name']);
602603
}
603604
});
604605

606+
it('chunks a cursor directly, without iterating it', function () {
607+
Item::insert([
608+
['name' => 'fork'],
609+
['name' => 'spoon'],
610+
['name' => 'spork'],
611+
]);
612+
613+
// Chained straight off cursor() rather than iterated - the call shape that
614+
// failed with "Call to undefined method Generator::chunk()".
615+
$chunks = Item::orderBy('name.keyword', 'asc')->cursor()->chunk(2);
616+
617+
expect($chunks->map->count()->all())->toBe([2, 1])
618+
->and($chunks->first()->pluck('name')->all())->toBe(['fork', 'spoon']);
619+
});
620+
605621
it('increments multiple fields simultaneously', function () {
606622
User::insert([
607623
['name' => 'John Doe', 'age' => 30, 'note' => 5],

0 commit comments

Comments
 (0)