Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(socketio): Async adapter #395

Open
wants to merge 33 commits into
base: feat-adapter-rework
Choose a base branch
from
Open

Conversation

Totodore
Copy link
Owner

@Totodore Totodore commented Nov 5, 2024

Motivation

Transform the Adapter trait to return impl Future + Send so that it is possible to implement adapters with other systems (redis, mongo, ...).

Solution

  • The Adapter returns impl Future + Send for all the methods that may need to communicate remotely (e.g emit, emit_with_ack on the BroadcastOperators and SocketIo structs).
  • The base adapter logic has been moved to the socketioxide_core trait as well as some error types. So that new adapters only need to depends on socketioxide_core.
  • Packet and AckError implement Serialize in order to send them to remote nodes.
  • ParserError is now simply a wrapper around a Box<impl StdError> so that it is easy to impl serde on it. Therefore the DecodeError and EncodeError GAT have been removed.
  • A new adapter State GAT allow any Adapter implementors to share a common state between all adapters of a node. It can be useful to share a connection to a remote server (redis, mongo, ...).
  • A new AckStream GAT allow the adapter to merge a stream from the local node with a remote stream of acknowledgments.

Remaining questions/issues:

  • Find a solution to initialize each namespace. Currently namespaces are initialized through a spawned task. But this leads to failed tests and the user can't get the error if the initialization failed.

To do:

  • Update documentation on operators. Some socket methods are sync and local whereas the same method is async on BroadcastOperators.

crates/socketioxide/src/operators.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/operators.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/operators.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/operators.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/operators.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/operators.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/operators.rs Fixed Show fixed Hide fixed
crates/socketioxide/tests/connect.rs Fixed Show fixed Hide fixed
* Remove Sync bound on adapter error
* Add an AdapterCtr trait to keep state
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clippy found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@Totodore Totodore mentioned this pull request Nov 21, 2024
This will allow adapter implementors to use the `LocalAdapter` as a parent.
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/adapter.rs Fixed Show fixed Hide fixed
This will allow to share errors between nodes.
The parser error is also now a Boxed error in order to have multiple dyn possible implementations;
@Totodore Totodore marked this pull request as ready for review December 15, 2024 14:38
crates/socketioxide/src/client.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/client.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/io.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/io.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/io.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/io.rs Fixed Show fixed Hide fixed
* return broadcast ack count
* remove empty rooms

#[inline(always)]
fn into_room_iter(self) -> Self::IntoIter {
self.into_iter().map(|i| Cow::Borrowed(*i))

Check warning

Code scanning / clippy

this .into_iter() call is equivalent to .iter() and will not consume the slice Warning

this .into\_iter() call is equivalent to .iter() and will not consume the slice

#[inline(always)]
fn into_room_iter(self) -> Self::IntoIter {
self.into_iter().map(|i| Cow::Borrowed(*i))

Check warning

Code scanning / clippy

this .into_iter() call is equivalent to .iter() and will not consume the slice Warning

this .into\_iter() call is equivalent to .iter() and will not consume the slice
crates/socketioxide/src/ns.rs Fixed Show fixed Hide fixed
crates/socketioxide/src/ns.rs Fixed Show fixed Hide fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant