Sync Reverb and Scout updates - #640
Conversation
laravel/reverb PR 406 closes the connection when a WebSocket handshake fails. Swoole already does this: it closes the connection after any non-101 response to an upgrade request, and an invalid key already gets a 400. The handshake never checked Sec-WebSocket-Version, though, so a client asking for an unsupported version got a 101 instead of RFC 6455's 426. The websocket-server handshake now rejects it after the key check with a 426 carrying Upgrade, Connection and Sec-WebSocket-Version 13, as Ratchet's negotiator does for upstream. Upstream's two handshake tests are ported to the Reverb integration ServerTest with their five-second client timeout, and ServerHandshakeTest covers the rejection before routing. Handshake helpers in the tests now send the version header a real client sends. Upstream reference: laravel/reverb main at 74c8c4082c. Validation: the changed test files, the WebSocketServer, Sentry, Foundation HTTP and Reverb unit and integration suites, formatting and PHPStan pass.
ClientEventTest's unsupported-message test expected hydratedConnections() never to be called, but the channel connection manager has no such method, so the expectation could never fail. laravel/reverb PR 407 asserts all() is never called instead, and the test now does the same. PR 407 converts upstream's tests from Mockery to Double. Hypervel keeps Mockery and doesn't port the exact call counts, which only constrain internal lookups. It also keeps the all() and find() stubs that PR 407 removed from the members-mode and none tests: Double fails on unused expectations, while Mockery's spy allows them, and the stubs are what let those tests' assertNothingReceived() checks catch a forwarded message. Upstream reference: laravel/reverb main at 74c8c4082c. Validation: the changed test file and the Reverb unit suite pass.
Signed HTTP API requests were accepted whatever their auth_timestamp, so a captured request could be replayed indefinitely. Following laravel/reverb commit 2f8a121813, the controller now rejects a request whose timestamp is missing or more than 600 seconds from the current time, after the signature itself is verified. Hypervel passes the verified query to the check instead of reading it from controller state. The signed-request test helpers take upstream's optional timestamp. ChannelsControllerTest ports the expired and future rejection test and adds a correctly signed request without a timestamp. Upstream reference: laravel/reverb main at 74c8c4082c. Validation: the changed test files, the Reverb unit and integration suites, formatting and PHPStan pass.
laravel/reverb PR 408 fixes presence channels when scaling is enabled.
Most of it is already part of Hypervel's design: member events are
routed as internal events, so other servers don't cache them;
SharedState atomically decides each user's first and last connection
across workers and servers; and presence data is gathered from every
worker or server and merged into one unique list. Upstream's
presence_connections metric, subscription timestamps and prefix-based
internal routing have no Hypervel counterpart.
Two fixes are ported into the subscription's presence data:
- A member without user_info made the subscription fail with an
ErrorException, and a member whose user_info was {} was sent as [],
because subscription data is decoded as an array. Both are now sent
as {}, as upstream's merge does.
- A failed presence gather propagated after the subscription was
committed, so the client never got its subscription_succeeded. The
failure is now reported and the subscription is answered with this
worker's members. Cancellation still propagates.
Tests ported: the no-user-info data test (with an empty user_info as
well), the merged and unknown-channel presence cases in
MetricsHandlerTest using Hypervel's snapshot payloads, the presence
cache test for internal events, the findOrCreate change, and the three
Redis scaling tests in RedisServerTest, with member_removed merged into
the existing member notification test. EventHandlerTest covers the
gather fallback and cancellation.
Upstream reference: laravel/reverb main at 74c8c4082c.
Validation: the changed test files, the Reverb unit and integration
suites, formatting and PHPStan pass.
Hypervel's Typesense engine already defaults the import action to upsert and passes the configured action to the import, as laravel/scout #955 does, but only the default was tested. The partial-engine test helper now takes config values; its config stub previously returned the default for every key. TypesenseEngineTest ports upstream's emplace action test. Upstream reference: laravel/scout 11.x at ce2542f5a7. Validation: TypesenseEngineTest, formatting and PHPStan pass.
Hypervel's Scout jobs already read their retry, backoff and exception limits from config, fail on timeout by default and support unique indexing, but some of upstream's job tests were missing. RemoveFromSearchTest ports the no-config, timeout default and timeout opt-out tests from laravel/scout #962 and #1002, replacing timeout assertions folded into the config tests. From #996 it ports the exact unique ID test, adapted to Hypervel's sha256 key, and the different-models test; the existing order test takes upstream's name. MakeSearchableTest gains the different-models test. Upstream reference: laravel/scout 11.x at ce2542f5a7. Validation: both test files, formatting and PHPStan pass.
Hypervel's Builder and engines already support where() with a comparison operator, as laravel/scout #969 added, but its per-operator tests were missing. DatabaseEngineTest ports the >, <, >=, <= and != tests, and its existing same-field comparison test takes upstream's name. The Meilisearch, Typesense and Algolia filtering integration tests ran one combined > and != query. They now run upstream's shared comparison cases, including the fixes from #976 and #978, which filter a typed numeric field and make the two-result assertions order-independent. Typesense filters its int32 ranking field, since its id is a string, and Algolia keeps its escaped-string case. Upstream reference: laravel/scout 11.x at ce2542f5a7. Validation: DatabaseEngineTest and the Meilisearch and Typesense filtering integration tests pass against those services; the Algolia integration test was not run locally because no Algolia credentials are configured. Formatting and PHPStan pass.
laravel/scout #1011 tests that a collection search for null returns
every model. In Hypervel, search() and the Builder constructor only
accepted a string, so Model::search($request->query('q')) threw a
TypeError when the request had no q parameter. Both now accept a
nullable query, and the Builder stores null as an empty string so
engines keep receiving a string query.
The collection engine already treated only an empty string as an empty
query, so searches for "0" and whitespace worked. CollectionEngineTest
ports upstream's null, whitespace and paginate-for-zero tests, and the
existing zero test takes upstream's name. The same file also ports
#969's collection-engine >, <, >=, <= and != tests, and its existing
same-field comparison test takes upstream's name.
Upstream reference: laravel/scout 11.x at ce2542f5a7.
Validation: CollectionEngineTest, the Scout test suite, formatting and
PHPStan pass.
Hypervel's Algolia engine already compiles boolean and inequality filters as laravel/scout #1005 does, but its test didn't cover boolean values in whereIn() and whereNotIn(). The existing boolean and inequality test now adds upstream's cases, asserting OR for inclusion and AND for exclusion with boolean literals. Upstream reference: laravel/scout 11.x at ce2542f5a7. Validation: AlgoliaEngineTest, formatting and PHPStan pass.
The Scout README listed internal changes, enhancements and fixes as differences, missed coroutine-local sync pausing, and didn't follow the package README layout. It now links the documentation and lists only the public differences that ported Laravel code has to account for: Algolia 4 only, numeric Algolia filter values, indexing without a queue, coroutine-local search-sync pausing, the tenant token method's arguments and the prefix requirement for deleting all indexes. Upstream reference: laravel/scout 11.x at ce2542f5a7.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: hypervel/components/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (28)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request updates Reverb presence snapshots and signed-request timestamp checks, adds WebSocket version validation, and changes Scout query typing and documentation. It also expands tests for these behaviors and for search-engine filters and job identifiers. ChangesReverb presence handling
Reverb request timestamp validation
WebSocket handshake version validation
Scout query and engine behavior
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No concrete issue remains that calls for a fix before merging; the change is ready subject to normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Request validation becomes stricter, and no new security bypass was identified. Presence responses can now be incomplete during outages, while compatibility with application-specific callers and recovery behavior remains partly unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 20.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 88 functions across 27 files. (1 skipped: 1 unsupported.) Full details: Description checkExplanation The description provides detailed change scope, supporting tests, and verification results, but it does not complete the required contribution-type and submission checkboxes or list exact verification commands. It also explicitly describes the pull request as a porting or synchronization PR, which the repository template says not to open. Resolution Select the applicable contribution type, complete all required checklist items, list the exact commands and results including the required composer fix run, and submit the work through the repository’s missing upstream functionality process if it is a porting or synchronization change.
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@cubic-dev-ai review |
@binaryfire I have started the AI code review. It will take a few minutes to complete. |
PR Summary by QodoSync Reverb and Scout behavior with upstream releases
AI Description
Diagram
High-Level Assessment
Files changed (28)
|
Code Review by Qodo
1. Models that override search fail to load
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 28 files
Confidence score: 3/5
- Existing
SearchableInterfaceimplementations that keepstring $querycan fail to load after this signature change. Add an upgrade note telling implementers to widen the parameter to?string. - In
SearchableInterface,search(null)can passnullto custom builders that require a string and throw aTypeError. Normalize the query to''before resolving the builder. - The differences section in
src/scout/README.mdno longer mentions theremoveAllFromSearch()force-flag difference, so readers may miss that this method still differs from Laravel’s parameterless version. Restore that note. - The Meilisearch note in
src/scout/README.mdleaves out the search rules parameter togenerateTenantToken(), which could mislead readers about the method signature.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/Integration/Reverb/ServerTest.php">
<violation number="1" location="tests/Integration/Reverb/ServerTest.php:75">
P3: This test verifies only the 426 status code and not the `Sec-WebSocket-Version` response header, which RFC 6455 §4.2.2 requires on a version-rejection handshake and which the server does send (src/websocket-server/src/Server.php throws the 426 with `Sec-WebSocket-Version => Security::VERSION`). Since the PR highlights the RFC 6455 426 behavior, assert the header (plus the `Upgrade: websocket`) so a regression that drops it is caught.</violation>
</file>
<file name="src/scout/README.md">
<violation number="1" location="src/scout/README.md:8">
P3: The differences rewrite drops the documented `Searchable::removeAllFromSearch()` force-flag difference, but the method still deviates from Laravel's parameterless `removeAllFromSearch()`: `src/scout/src/Searchable.php:316` is `removeAllFromSearch(bool $force = false)`, and the `scout:flush` command enables it via `Scout::guardModelFlush`. That is a deliberate lasting public-contract difference that the README should keep (or point to the docs section covering it) so porters don't miss the force behavior.</violation>
<violation number="2" location="src/scout/README.md:14">
P3: This bullet describes the Meilisearch client method imprecisely: it says the client's `generateTenantToken()` "takes the UID first and an options array," but the signature also takes the search rules as the second positional argument (`$uid, $searchRules, $options`), and the engine still passes the call through to the client at the end (after validating and reordering arguments). Tighten the wording so the contrast is accurate.</violation>
</file>
<file name="src/scout/src/Contracts/SearchableInterface.php">
<violation number="1" location="src/scout/src/Contracts/SearchableInterface.php:30">
P2: Normalize `$query` to `''` before resolving `$scoutBuilder`; `search(null)` otherwise passes null to custom builders whose constructor requires `string` and throws a `TypeError`.</violation>
<violation number="2" location="src/scout/src/Contracts/SearchableInterface.php:30">
P2: Existing `SearchableInterface` implementations with `string $query` now fail class loading. Add an upgrade note requiring those implementations to widen the parameter to `?string`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * @return Builder<Model&static> | ||
| */ | ||
| public static function search(string $query = '', ?Closure $callback = null): Builder; | ||
| public static function search(?string $query = '', ?Closure $callback = null): Builder; |
There was a problem hiding this comment.
P2: Normalize $query to '' before resolving $scoutBuilder; search(null) otherwise passes null to custom builders whose constructor requires string and throws a TypeError.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/scout/src/Contracts/SearchableInterface.php, line 30:
<comment>Normalize `$query` to `''` before resolving `$scoutBuilder`; `search(null)` otherwise passes null to custom builders whose constructor requires `string` and throws a `TypeError`.</comment>
<file context>
@@ -27,7 +27,7 @@ interface SearchableInterface
* @return Builder<Model&static>
*/
- public static function search(string $query = '', ?Closure $callback = null): Builder;
+ public static function search(?string $query = '', ?Closure $callback = null): Builder;
/**
</file context>
There was a problem hiding this comment.
Keeping this as is. Laravel's search() also passes the raw query to static::$scoutBuilder. Since search() returns a Builder, a custom builder is a Builder subclass, and Builder's constructor accepts null and stores it as an empty query. Normalizing the query in search() as well would duplicate that.
| * @return Builder<Model&static> | ||
| */ | ||
| public static function search(string $query = '', ?Closure $callback = null): Builder; | ||
| public static function search(?string $query = '', ?Closure $callback = null): Builder; |
There was a problem hiding this comment.
P2: Existing SearchableInterface implementations with string $query now fail class loading. Add an upgrade note requiring those implementations to widen the parameter to ?string.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/scout/src/Contracts/SearchableInterface.php, line 30:
<comment>Existing `SearchableInterface` implementations with `string $query` now fail class loading. Add an upgrade note requiring those implementations to widen the parameter to `?string`.</comment>
<file context>
@@ -27,7 +27,7 @@ interface SearchableInterface
* @return Builder<Model&static>
*/
- public static function search(string $query = '', ?Closure $callback = null): Builder;
+ public static function search(?string $query = '', ?Closure $callback = null): Builder;
/**
</file context>
There was a problem hiding this comment.
No note needed. Laravel's search($query = '', $callback = null) has no parameter type, so overrides ported from Laravel don't declare string $query, and an untyped $query is compatible with ?string. Only an override copied from Hypervel's earlier signature is affected, and the upgrade guide already recommends moving 0.3 code into a fresh 0.4 application and updating it to the Laravel-style APIs rather than listing individual signature changes.
| ]); | ||
| $client->get('/app/' . $this->appKey); | ||
|
|
||
| $this->assertSame(426, $client->getStatusCode()); |
There was a problem hiding this comment.
P3: This test verifies only the 426 status code and not the Sec-WebSocket-Version response header, which RFC 6455 §4.2.2 requires on a version-rejection handshake and which the server does send (src/websocket-server/src/Server.php throws the 426 with Sec-WebSocket-Version => Security::VERSION). Since the PR highlights the RFC 6455 426 behavior, assert the header (plus the Upgrade: websocket) so a regression that drops it is caught.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/Integration/Reverb/ServerTest.php, line 75:
<comment>This test verifies only the 426 status code and not the `Sec-WebSocket-Version` response header, which RFC 6455 §4.2.2 requires on a version-rejection handshake and which the server does send (src/websocket-server/src/Server.php throws the 426 with `Sec-WebSocket-Version => Security::VERSION`). Since the PR highlights the RFC 6455 426 behavior, assert the header (plus the `Upgrade: websocket`) so a regression that drops it is caught.</comment>
<file context>
@@ -41,6 +43,40 @@ public function testFailsToConnectWithInvalidAppKey(): void
+ ]);
+ $client->get('/app/' . $this->appKey);
+
+ $this->assertSame(426, $client->getStatusCode());
+
+ $client->close();
</file context>
| $this->assertSame(426, $client->getStatusCode()); | |
| $this->assertSame(426, $client->getStatusCode()); | |
| $this->assertSame('13', $client->getHeaders()['sec-websocket-version'] ?? null); |
There was a problem hiding this comment.
Fixed in 4f1f7aa. The live test now also asserts Sec-WebSocket-Version: 13. A running Reverb server renders handshake errors through the application's exception handler, while the unit tests cover the WebSocket server's own handler, so this checks the header on the production path.
I didn't add a separate Upgrade assertion. The server sets Upgrade, Connection and Sec-WebSocket-Version together on the same exception, ServerHandshakeTest already asserts all three, and the version header check shows those headers reach the client through the application's handler.
| - `Searchable::removeAllFromSearch()` accepts an optional force flag, which the explicit `scout:flush` command enables. | ||
| - Without a queue, indexing is deferred until after the HTTP response is sent, and runs immediately outside a request. | ||
| - Pausing search syncing with `withoutSyncingToSearch()` or `disableSearchSyncing()` applies only to the current coroutine, so other requests keep indexing. See [Pausing Indexing](https://hypervel.org/docs/scout#pausing-indexing). | ||
| - `MeilisearchEngine::generateTenantToken()` takes the search rules, the parent key's UID and the key itself, instead of passing the call through to the Meilisearch client, whose method takes the UID first and an options array. See [Tenant Tokens](https://hypervel.org/docs/scout#meilisearch-tenant-tokens). |
There was a problem hiding this comment.
P3: This bullet describes the Meilisearch client method imprecisely: it says the client's generateTenantToken() "takes the UID first and an options array," but the signature also takes the search rules as the second positional argument ($uid, $searchRules, $options), and the engine still passes the call through to the client at the end (after validating and reordering arguments). Tighten the wording so the contrast is accurate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/scout/README.md, line 14:
<comment>This bullet describes the Meilisearch client method imprecisely: it says the client's `generateTenantToken()` "takes the UID first and an options array," but the signature also takes the search rules as the second positional argument (`$uid, $searchRules, $options`), and the engine still passes the call through to the client at the end (after validating and reordering arguments). Tighten the wording so the contrast is accurate.</comment>
<file context>
@@ -3,16 +3,15 @@ Scout for Hypervel
-- `Searchable::removeAllFromSearch()` accepts an optional force flag, which the explicit `scout:flush` command enables.
+- Without a queue, indexing is deferred until after the HTTP response is sent, and runs immediately outside a request.
+- Pausing search syncing with `withoutSyncingToSearch()` or `disableSearchSyncing()` applies only to the current coroutine, so other requests keep indexing. See [Pausing Indexing](https://hypervel.org/docs/scout#pausing-indexing).
+- `MeilisearchEngine::generateTenantToken()` takes the search rules, the parent key's UID and the key itself, instead of passing the call through to the Meilisearch client, whose method takes the UID first and an options array. See [Tenant Tokens](https://hypervel.org/docs/scout#meilisearch-tenant-tokens).
+- `scout:delete-all-indexes` refuses to run without a configured Scout prefix unless you pass `--force`.
+
</file context>
| - `MeilisearchEngine::generateTenantToken()` takes the search rules, the parent key's UID and the key itself, instead of passing the call through to the Meilisearch client, whose method takes the UID first and an options array. See [Tenant Tokens](https://hypervel.org/docs/scout#meilisearch-tenant-tokens). | |
| +`MeilisearchEngine::generateTenantToken()` takes the search rules, the parent key's UID and the key itself, and validates and reorders them before delegating to the Meilisearch client's `generateTenantToken()`, which takes the UID first followed by the search rules and an options array. See [Tenant Tokens](https://hypervel.org/docs/scout#meilisearch-tenant-tokens). |
There was a problem hiding this comment.
Fixed in 0c706dc. The entry now describes both methods accurately: Hypervel's engine takes the search rules, the parent key's UID, the key and an optional expiry, and calls the client with them in the client's order. Laravel's engine forwards the call unchanged to the Meilisearch client, whose method takes the UID, the search rules and an options array.
|
|
||
| Differences From Laravel | ||
| --- | ||
| ## Differences From Laravel |
There was a problem hiding this comment.
P3: The differences rewrite drops the documented Searchable::removeAllFromSearch() force-flag difference, but the method still deviates from Laravel's parameterless removeAllFromSearch(): src/scout/src/Searchable.php:316 is removeAllFromSearch(bool $force = false), and the scout:flush command enables it via Scout::guardModelFlush. That is a deliberate lasting public-contract difference that the README should keep (or point to the docs section covering it) so porters don't miss the force behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/scout/README.md, line 8:
<comment>The differences rewrite drops the documented `Searchable::removeAllFromSearch()` force-flag difference, but the method still deviates from Laravel's parameterless `removeAllFromSearch()`: `src/scout/src/Searchable.php:316` is `removeAllFromSearch(bool $force = false)`, and the `scout:flush` command enables it via `Scout::guardModelFlush`. That is a deliberate lasting public-contract difference that the README should keep (or point to the docs section covering it) so porters don't miss the force behavior.</comment>
<file context>
@@ -3,16 +3,15 @@ Scout for Hypervel
-Differences From Laravel
----
+## Differences From Laravel
- Algolia 4 is the only supported Algolia client.
</file context>
There was a problem hiding this comment.
Keeping this out of the README. Its differences section only lists differences that ported Laravel code has to account for. removeAllFromSearch() still works with no arguments, as in Laravel. The optional force argument only matters to an application that registers a model-flush guard, and the Scout documentation covers it under Removing Records and Customizing Scout Lifecycles.
Two timing assertions occasionally failed in the Redis Cluster CI runs. The funnel lease refresh test slept 1.1 seconds after acquiring a three-second lease, then expected the refreshed lifetime to be longer than the remaining one. The database and file stores report lifetimes in whole seconds, rounding the deadline up and the current time down, so when a second ended between the two reads both values could be 3. Sleeping 2.1 seconds leaves at most 2 seconds before the refresh and at least 3 after it. The duration limiter's window runs from the current whole second to that second plus the decay, while each attempt is compared with the fractional current time. A one-second window opened late in a second only lasts for the rest of that second, so an immediate second attempt could land in a new window and succeed. This is normal fixed-window behavior, shared with Laravel's limiter, so the limiter is unchanged. The two tests that expect an immediate second attempt in a one-second window to fail now start just after a whole second. Validation: DurationLimiterIntegrationTest and the cache funnel tests pass against Redis.
The live Reverb server test for an unsupported WebSocket version only checked the 426 status. A running Reverb server renders handshake errors through the application's exception handler, while the unit tests that check the Sec-WebSocket-Version header cover the WebSocket server's own handler. The live test now also asserts the Sec-WebSocket-Version: 13 header that RFC 6455 requires on this response, so the production path is covered. Upstream reference: laravel/reverb main at 74c8c4082c. Validation: ServerTest passes against a Reverb test server.
The entry left out the search rules when describing the Meilisearch client's generateTenantToken() and didn't mention the engine's optional expiry. It now lists the arguments of both methods: Hypervel's engine takes the search rules, the parent key's UID, the key and an optional expiry, while Laravel's engine forwards the call to the client, whose method takes the UID, the search rules and an options array. Upstream reference: laravel/scout 11.x at ce2542f5a7.
1 participant
This completes the Reverb update to laravel/reverb
main, following the first set of changes in #639. It also brings Scout up to laravel/scout11.x, except for semantic and hybrid search (laravel/scout#1007, laravel/scout#1008, laravel/scout#1009 and laravel/scout#1012), which follow in their own PR. Hypervel already had most of Scout's other changes, so the Scout work is mainly upstream's missing tests, plus a fix for null search queries.Upstream Updates
Reverb
Sec-WebSocket-Version, so a client asking for an unsupported version was upgraded anyway. It now gets RFC 6455's 426 response withSec-WebSocket-Version: 13, as Ratchet's negotiator sends for upstream. Upstream's two handshake tests are ported, and the handshake tests now send the version header that real clients send.all()is never called, as upstream's does.auth_timestampis missing or more than 600 seconds from the current time. Hypervel accepted any timestamp, so a captured request could be replayed indefinitely. Upstream's test for expired and future timestamps is ported, along with a check that a correctly signed request without a timestamp is rejected.SharedStatedecides each user's first and last connection atomically across workers and servers, and presence members are gathered from every worker and server and merged. Two fixes are ported. A member withoutuser_infomade the subscription fail, and emptyuser_infowas sent as[]; both are now sent as{}. And if gathering members from other workers or servers failed after the subscription was committed, the client never got its subscription confirmation. The failure is now reported and the client gets this worker's members. Upstream's tests are ported, including its three Redis scaling tests.Scout
upsert. Hypervel already did, but only the default was tested. Upstream'semplacetest is ported, and the test helper's config stub, which returned the default for every key, now takes config values.where($field, $operator, $value), which Hypervel already supported. The collection and database engine tests for>,<,>=,<=and!=are ported. The Meilisearch, Typesense and Algolia integration tests ran one combined query. They now run upstream's comparison cases with the changes from laravel/scout#976 and laravel/scout#978, which filter a typed numeric field and accept two results in either order. Typesense filters its integerrankingfield, since itsidis a string, and Algolia keeps its escaped-string case.whereInandwhereNotIncases."0", which Hypervel already handled. Upstream's new test also searches fornull, whichsearch()rejected:Model::search($request->query('q'))threw aTypeErrorwhen the request had noq.search()and theBuilderconstructor now acceptnull, and the builder stores it as an empty query. Upstream's null, whitespace and paginate-for-zero tests are ported.Additional Hypervel Fixes
The changed tests, the Reverb unit and integration suites, the WebSocket server tests, the Scout unit and feature tests, the Meilisearch and Typesense filtering integration tests, formatting and static analysis pass locally. The Algolia integration tests weren't run locally because they need Algolia credentials, and CI runs them only when those credentials are configured.
Summary by CodeRabbit
null; they’re treated as empty searches.Note
Sync Reverb and Scout updates: presence fallback, signature timestamps, nullable Scout queries
searchqueries now accept null, normalized to an empty query inBuilderand allowed bySearchableInterface(Builder.php).Macroscope summarized 0c706dc.