refactor(core): move remote request handling into core behind remote-adapter feature - #783
Open
LiamCarPer wants to merge 1 commit into
Open
Conversation
…adapter feature Moves the duplicated remote request handling from the redis, postgres and mongodb adapters into socketioxide-core behind the existing remote-adapter feature flag. - RemoteRequestHandler trait with default implementations of recv_req and all the recv_* handlers (broadcast, broadcast_with_ack, rooms, add/del sockets, fetch sockets, disconnect sockets). Adapters only provide their local adapter, send_res and an optional recv_heartbeat hook. - insert_response_handler/wait_responses helpers covering the get_res channel setup, response type filter, take/timeout and handler cleanup. Some tiny differences were normalized on purpose: send_res now takes (req_id, req_origin) everywhere with the strictest bounds, a request without options warns and is skipped for all adapters, the loopback check is done in the shared dispatch and logs use the same style. The postgres adapter get_res now also wraps the response stream in a DropStream, fixing a leak of response handler entries for rooms and fetch sockets requests. No behavior or wire-format change. Closes Totodore#774
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
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.
Part of #727.
Motivation
The redis, postgres and mongodb adapters each implement the same request handling: a near identical
recv_reqdispatch plus the samerecv_*handlers (broadcast, broadcast_with_ack, rooms, add/del sockets, fetch sockets, disconnect sockets), and the sameget_resskeleton (channel setup, response filter, take/timeout).Solution
Added a
RemoteRequestHandlertrait insocketioxide-corebehind the existingremote-adapterfeature flag, with default implementations of the dispatch and of all the handlers. Each adapter only implements:local(), to access itsCoreLocalAdaptersend_res(req_id, req_origin, res), to send responsesrecv_heartbeat(), a no-op by default. Postgres and mongodb delegate to theHeartbeatSenderlogic from refactor(core): move heartbeat/liveness logic into core behind remote-adapter feature #768.Redis and mongodb keep a tiny wrapper that decodes the raw driver item and delegates, postgres calls the dispatch directly from its notification pipeline.
Also added
insert_response_handlerandwait_responseshelpers covering the identicalget_reschannel setup, response type filter, take/timeout and handler cleanup.Some tiny differences were normalized on purpose:
send_resnow takes(req_id, req_origin)everywhere with the strictest bounds (T: Serialize + Debug + Send + 'static, future+ Send + 'static), redis's inverted order is handled inside its implThe postgres
get_resnow also wraps the response stream in aDropStream, fixing a leak of response handler entries forroomsandfetch_socketsrequests.No behavior or wire-format change.
Tests
The existing adapter tests (fixture based, no real DB needed) cover every moved handler for the three adapters. Added 6 unit tests in core for the default dispatch: add/del sockets, heartbeat hook, loopback ignore, missing options, and the rooms / broadcast with ack responses.