Sync Reverb and Scout updates - #49
Merged
Merged
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.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: hypervel/components-backup/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
PR Summary by QodoSync Reverb and Scout fixes with upstream coverage
AI Description
Diagram
High-Level Assessment
Files changed (28)
|
Code Review by Qodo
1. Presence members lose falsy profile data
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comments Outside DiffThese findings could not be posted inline.
|
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This completes the Reverb update to laravel/reverb
mainat74c8c4082c. It also brings Scout up to laravel/scout11.xatce2542f5a7, except for semantic and hybrid search (laravel/scout pull requests 1007, 1008, 1009 and 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.2f8a121813rejects signed HTTP API requests whoseauth_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 pull requests 976 and 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 cubic
Syncs
laravel/reverbandlaravel/scoutto their latest upstream releases, with most changes landing as ported tests since Hypervel already had most of the implementations.auth_timestampis missing or more than 600 seconds from the current time, preventing indefinite replay of captured requests.Sec-WebSocket-Versionwith RFC 6455's 426 response, after the key check. The live server test now asserts theSec-WebSocket-Version: 13header.{}for members withoutuser_infoand answer with local members when presence gathering from other workers or servers fails.search()and the ScoutBuildernow acceptnullqueries, soModel::search($request->query('q'))no longer throws aTypeErrorwhenqis absent.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.
Written for commit 0c706dc. Summary will update on new commits.
Note
Add WebSocket version validation, Pusher signature timestamp checks, and presence fallback in Reverb/Scout sync
Server::onHandshakein Server.php now rejects missing or unsupported WebSocket versions before routing, returning HTTP 426 with upgrade headers.SIGNATURE_TOLERANCE;verifySignatureTimestamprejects signed requests with missing, nonnumeric, or out-of-window timestamps with a 401 error.Builder::__construct,Searchable::search, andSearchableInterface::searchnormalize null to an empty string. Scout README is updated to reflect framework differences.Macroscope summarized 0c706dc.