Skip to content

Commit 25f630e

Browse files
committed
returns MetaDTO instead of ?MetaDTO
1 parent b5a4351 commit 25f630e

5 files changed

Lines changed: 197 additions & 27 deletions

File tree

CHANGELOG.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ This release is compatible with Laravel 11, 12 & 13, and with Elasticsearch 8 &
1010

1111
- **Elasticsearch 9 support** - the full suite runs green against ES 8.18 and 9.5, and CI now tests
1212
both on every push. No code changes were needed: the `^8.17` client talks to both server lines.
13+
- **`ElasticCollection::hasQueryMeta()`** - whether a collection carries the meta of the query that
14+
built it. False for the collections Eloquent builds on its own: `newCollection()`, `hydrate()`,
15+
and anything derived through `map()`, `filter()` or `values()`.
1316

1417
### Fixed
1518

@@ -19,13 +22,28 @@ This release is compatible with Laravel 11, 12 & 13, and with Elasticsearch 8 &
1922
[laravel-opensearch#28](https://github.com/pdphilip/laravel-opensearch/pull/28)
2023
- **`cursor()` crashed on a fresh connection** - `Processor::getRawResponse()` dereferenced null when
2124
a cursor was the first query on its connection (`Call to a member function asArray() on null`)
25+
- **`paginate()` lost its query meta when nothing matched** - Laravel skips the query entirely once
26+
the count comes back as zero, so an empty page arrived as a bare collection and `getQueryMeta()`
27+
on it threw `Method Illuminate\Database\Eloquent\Collection::getQueryMeta does not exist`. The
28+
count query's meta is now carried onto the empty page: `getTotalHits()` reports `0`, and it still
29+
honours `withTrackTotalHits()` for indices past the 10k window
30+
- **Meta getters fataled on collections Eloquent built itself** - `ElasticCollection::$meta` was a
31+
typed property that nothing initialised, so `getQueryMeta()`, `getTook()`, `getDsl()` and the rest
32+
threw `Typed property ElasticCollection::$meta must not be accessed before initialization` on any
33+
collection that did not come from a query. They now fall back to an empty `QueryMeta`, which
34+
reports its unknowns as `-1`. Note that nullsafe could not guard this: the error came from the
35+
property access, so `$collection?->getQueryMeta()` fataled too
36+
- **`Query\Builder::get()` could hand a null `MetaDTO` to `setQueryMeta()`** - it read the
37+
`$metaTransfer` property directly instead of the accessor that initialises it
2238

2339
### Changed
2440

2541
- `cursor()` returns `LazyCollection` instead of `Generator`. `foreach` is unaffected; anything
2642
type-hinting `Generator` or `Iterator` needs updating
2743
- A cursor is re-iterable - a second pass opens a fresh scroll instead of throwing
2844
`Cannot rewind a generator`
45+
- `Query\Builder::getMetaTransfer()` (internal) returns `MetaDTO` instead of `?MetaDTO` - it
46+
initialises on demand and never actually returned null
2947

3048
> **Laravel 11 notice:** three advisories now cover the whole 11.x branch with no 11.x release that
3149
> clears them, and Composer 2.9+ blocks advisory-affected versions while resolving. On Laravel 11,

src/Eloquent/Builder.php

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,49 @@ public function chunkByPit($count, callable $callback, $keepAlive = '1m'): bool
318318
return true;
319319
}
320320

321+
/**
322+
* {@inheritdoc}
323+
*
324+
* Laravel skips the query entirely when the count comes back as zero and
325+
* returns a bare collection. Elasticsearch still ran a search to produce
326+
* that zero, so the count query's meta is carried over rather than lost -
327+
* without it getQueryMeta() on an empty page has nothing to report.
328+
*
329+
* @param Closure|int|null $perPage
330+
* @param Closure|int|null $total
331+
*/
332+
public function paginate($perPage = null, $columns = ['*'], $pageName = 'page', $page = null, $total = null)
333+
{
334+
$page = $page ?: Paginator::resolveCurrentPage($pageName);
335+
$countQuery = $this->toBase();
336+
$total = value($total) ?? $countQuery->getCountForPagination();
337+
$perPage = value($perPage, $total) ?: $this->model->getPerPage();
338+
339+
$results = $total
340+
? $this->forPage($page, $perPage)->get($columns)
341+
: $this->emptyPageCollection($countQuery);
342+
343+
return $this->paginator($results, $total, $perPage, $page, [
344+
'path' => Paginator::resolveCurrentPath(),
345+
'pageName' => $pageName,
346+
]);
347+
}
348+
349+
/**
350+
* The collection for a page that matched nothing, carrying the meta of the
351+
* count query where the model's collection can hold it.
352+
*/
353+
protected function emptyPageCollection(QueryBuilder $countQuery): Collection
354+
{
355+
$collection = $this->model->newCollection();
356+
357+
if ($collection instanceof ElasticCollection) {
358+
$collection->setQueryMeta($countQuery->getMetaTransfer());
359+
}
360+
361+
return $collection;
362+
}
363+
321364
/**
322365
* Using Laravel base method name rather
323366
*

src/Eloquent/ElasticCollection.php

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace PDPhilip\Elasticsearch\Eloquent;
44

5-
use Illuminate\Contracts\Support\Arrayable;
65
use Illuminate\Database\Eloquent\Collection;
76
use PDPhilip\Elasticsearch\Data\MetaDTO;
87
use PDPhilip\Elasticsearch\Data\QueryMeta;
@@ -15,16 +14,7 @@
1514
*/
1615
class ElasticCollection extends Collection
1716
{
18-
protected ?QueryMeta $meta;
19-
20-
/**
21-
* @param Arrayable<TKey, TModel>|iterable<TKey, TModel>|array<TKey|int, mixed>|null $items
22-
*/
23-
public function __construct($items = [])
24-
{
25-
parent::__construct($items);
26-
// $this->meta = new QueryMeta;
27-
}
17+
protected ?QueryMeta $meta = null;
2818

2919
public static function loadCollection(Collection $collection)
3020
{
@@ -45,56 +35,75 @@ public function setQueryMeta(MetaDTO $meta): self
4535
return $this;
4636
}
4737

38+
/**
39+
* Whether this collection carries the meta of the query that built it.
40+
*
41+
* False for the collections Eloquent builds on its own: newCollection(),
42+
* hydrate(), and anything derived through map(), filter() or values().
43+
*/
44+
public function hasQueryMeta(): bool
45+
{
46+
return $this->meta !== null;
47+
}
48+
49+
/**
50+
* Meta of the query that built this collection.
51+
*
52+
* Falls back to an empty QueryMeta, which reports its unknowns as -1, so
53+
* that a collection built without a query still answers every getter. The
54+
* fallback is not retained: reading meta that was never set must not make
55+
* hasQueryMeta() start answering true.
56+
*/
4857
public function getQueryMeta(): QueryMeta
4958
{
50-
return $this->meta;
59+
return $this->meta ?? new QueryMeta;
5160
}
5261

5362
public function getQueryMetaAsArray(): array
5463
{
55-
return $this->meta->toArray();
64+
return $this->getQueryMeta()->toArray();
5665
}
5766

5867
public function getDsl(): array
5968
{
6069
return [
61-
'query' => $this->meta->getQuery(),
62-
'dsl' => $this->meta->getDsl(),
70+
'query' => $this->getQueryMeta()->getQuery(),
71+
'dsl' => $this->getQueryMeta()->getDsl(),
6372
];
6473
}
6574

6675
public function getTook(): int
6776
{
68-
return $this->meta->getTook();
77+
return $this->getQueryMeta()->getTook();
6978
}
7079

7180
public function getShards(): mixed
7281
{
73-
return $this->meta->getShards();
82+
return $this->getQueryMeta()->getShards();
7483
}
7584

7685
public function getTotal(): int
7786
{
78-
return $this->meta->getTotal();
87+
return $this->getQueryMeta()->getTotal();
7988
}
8089

8190
public function getMaxScore(): string
8291
{
83-
return $this->meta->getMaxScore();
92+
return $this->getQueryMeta()->getMaxScore();
8493
}
8594

8695
public function getResults(): array
8796
{
88-
return $this->meta->getResults();
97+
return $this->getQueryMeta()->getResults();
8998
}
9099

91100
public function getPitId()
92101
{
93-
return $this->meta->getPitId();
102+
return $this->getQueryMeta()->getPitId();
94103
}
95104

96105
public function getAfterKey()
97106
{
98-
return $this->meta->getAfterKey();
107+
return $this->getQueryMeta()->getAfterKey();
99108
}
100109
}

src/Query/Builder.php

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ public function get($columns = ['*']): ElasticCollection|array
593593
$results = $this->getResultsOnce();
594594
$this->columns = $original;
595595
$collection = ElasticCollection::make($results);
596-
$collection->setQueryMeta($this->metaTransfer);
596+
$collection->setQueryMeta($this->getMetaTransfer());
597597

598598
return $collection;
599599
}
@@ -670,9 +670,11 @@ public function getCountForPagination($columns = ['*'])
670670
*/
671671
protected function runPaginationCountQuery($columns = ['_id'])
672672
{
673-
return $this->cloneWithout(['columns', 'orders', 'limit', 'offset'])
674-
->limit(1)
675-
->get($columns)->all();
673+
$countQuery = $this->cloneWithout(['columns', 'orders', 'limit', 'offset'])->limit(1);
674+
$results = $countQuery->get($columns)->all();
675+
$this->setMetaTransfer($countQuery->getMetaTransfer());
676+
677+
return $results;
676678
}
677679

678680
/**
@@ -1313,7 +1315,7 @@ public function setMetaTransfer(MetaDTO $metaTransfer): void
13131315
}
13141316

13151317
// @internal
1316-
public function getMetaTransfer(): ?MetaDTO
1318+
public function getMetaTransfer(): MetaDTO
13171319
{
13181320
if (! $this->metaTransfer) {
13191321
$this->metaTransfer = new MetaDTO([]);

tests/QueryMetaTest.php

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use PDPhilip\Elasticsearch\Data\QueryMeta;
6+
use PDPhilip\Elasticsearch\Eloquent\ElasticCollection;
7+
use PDPhilip\Elasticsearch\Tests\Models\User;
8+
9+
beforeEach(function () {
10+
User::executeSchema();
11+
12+
User::insert([
13+
['name' => 'John Doe', 'age' => 35, 'title' => 'admin'],
14+
['name' => 'Jane Doe', 'age' => 33, 'title' => 'admin'],
15+
['name' => 'Harry Hoe', 'age' => 13, 'title' => 'user'],
16+
['name' => 'Robert Roe', 'age' => 37, 'title' => 'user'],
17+
['name' => 'Mark Moe', 'age' => 23, 'title' => 'user'],
18+
]);
19+
});
20+
21+
it('carries the query meta on a page that has results', function () {
22+
$results = User::where('title', 'user')->paginate(2);
23+
24+
expect($results->getCollection())->toBeInstanceOf(ElasticCollection::class)
25+
->and($results->getCollection()->hasQueryMeta())->toBeTrue()
26+
->and($results->total())->toBe(3)
27+
->and($results->getQueryMeta()->getTotalHits())->toBe(3);
28+
});
29+
30+
it('carries the query meta on a page that matched nothing', function () {
31+
$results = User::where('age', 999)->paginate(2);
32+
33+
expect($results->getCollection())->toBeInstanceOf(ElasticCollection::class)
34+
->and($results->getCollection()->hasQueryMeta())->toBeTrue()
35+
->and($results->count())->toBe(0)
36+
->and($results->total())->toBe(0)
37+
->and($results->getQueryMeta())->toBeInstanceOf(QueryMeta::class)
38+
->and($results->getQueryMeta()->getTotalHits())->toBe(0)
39+
->and($results->getQueryMeta()->getTook())->toBeGreaterThanOrEqual(0);
40+
});
41+
42+
it('reports the full total on a page that matched nothing when tracking total hits', function () {
43+
$results = User::where('age', 999)->withTrackTotalHits()->paginate(2);
44+
45+
expect($results->total())->toBe(0)
46+
->and($results->getQueryMeta()->getTotalHits())->toBe(0);
47+
});
48+
49+
it('keeps the query meta through paginator transforms', function () {
50+
$emptyPage = User::where('age', 999)->paginate(2)
51+
->withQueryString()
52+
->through(fn ($user) => ['name' => $user->name]);
53+
54+
$fullPage = User::where('title', 'user')->paginate(2)
55+
->withQueryString()
56+
->through(fn ($user) => ['name' => $user->name]);
57+
58+
expect($emptyPage->getQueryMeta()->getTotalHits())->toBe(0)
59+
->and($fullPage->getQueryMeta()->getTotalHits())->toBe(3);
60+
});
61+
62+
it('answers every meta getter on a collection built without a query', function () {
63+
$collection = new ElasticCollection;
64+
65+
expect($collection->hasQueryMeta())->toBeFalse()
66+
->and($collection->getQueryMeta())->toBeInstanceOf(QueryMeta::class)
67+
->and($collection->getQueryMeta()->getTotalHits())->toBe(-1)
68+
->and($collection->getQueryMetaAsArray())->toBe([])
69+
->and($collection->getTook())->toBe(-1)
70+
->and($collection->getTotal())->toBe(-1)
71+
->and($collection->getMaxScore())->toBe('')
72+
->and($collection->getShards())->toBe([])
73+
->and($collection->getDsl())->toBe(['query' => '', 'dsl' => []])
74+
->and($collection->getResults())->toBe([])
75+
->and($collection->getAfterKey())->toBe([])
76+
->and($collection->getPitId())->toBeNull()
77+
->and($collection->hasQueryMeta())->toBeFalse();
78+
});
79+
80+
it('builds meta safe collections for models', function () {
81+
$newCollection = (new User)->newCollection();
82+
$hydrated = User::hydrate([['name' => 'John Doe']]);
83+
84+
expect($newCollection)->toBeInstanceOf(ElasticCollection::class)
85+
->and($newCollection->hasQueryMeta())->toBeFalse()
86+
->and($newCollection->getQueryMeta()->getTotalHits())->toBe(-1)
87+
->and($hydrated)->toBeInstanceOf(ElasticCollection::class)
88+
->and($hydrated->hasQueryMeta())->toBeFalse()
89+
->and($hydrated->getQueryMeta()->getTotalHits())->toBe(-1);
90+
});
91+
92+
it('keeps the query meta on a fetched collection', function () {
93+
$users = User::where('title', 'user')->get();
94+
95+
expect($users->hasQueryMeta())->toBeTrue()
96+
->and($users->getQueryMeta()->getTotalHits())->toBe(3)
97+
->and($users->getTook())->toBeGreaterThanOrEqual(0);
98+
});

0 commit comments

Comments
 (0)