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

Dynamic Namespaces #212

Open
FabianHummel opened this issue Dec 18, 2023 · 10 comments · May be fixed by #333
Open

Dynamic Namespaces #212

FabianHummel opened this issue Dec 18, 2023 · 10 comments · May be fixed by #333
Labels
enhancement New feature or request help wanted Extra attention is needed socketio-v4 It is only about the version 4 of the socket.io protocol socketio-v5 It is only about the version 5 of the socket.io protocol

Comments

@FabianHummel
Copy link

I have noticed that dynamic namespaces via regex are not yet implemented. Is this planned?

Is there a way for custom middleware to handle this in the meantime?

@FabianHummel FabianHummel added the enhancement New feature or request label Dec 18, 2023
@Totodore Totodore added the help wanted Extra attention is needed label Dec 18, 2023
@Totodore
Copy link
Owner

It is not planned for the moment. I'll do some research on it and maybe propose directions for anyone who would like to work on it.

@FabianHummel
Copy link
Author

FabianHummel commented Dec 19, 2023

Is it possible to work around this issue by using custom Middleware in axum or socketioxide to intercept the connection request and create a namespace with io.ns(request.path /*target path to create dynamically*/) and later delete it again when the client decides to close the session? Currently it only says "invalid namespace" when connecting - there must be a way to intercept that...

Or maybe preflight a connection attempt on the path "/", create the namespace there, and connect a second time then to the namespace, even though that seems quite cumbersome

@Totodore
Copy link
Owner

Totodore commented Dec 19, 2023

At any time you can debug the io struct and you should be able to get the map of the current registered namespaces.

I don't think I'll implement regex based matching or function base matching. If the user add a bad regex or a long-computation function it would slow down routing. I'd like to go the "axum-way" and use the matchit crate.

If anyone would like to work on this issue :

  • Replace the Hashmap struct in the client with the matchit::Router struct
  • Throw a panic in case of concurrent route at insertion.
  • Add a simple benchmark to verify that routing is constant with a lot of handlers
  • Propagate the parameters to the handler and add a Param extractor

@Totodore Totodore removed their assignment Dec 19, 2023
@Totodore Totodore added socketio-v4 It is only about the version 4 of the socket.io protocol socketio-v5 It is only about the version 5 of the socket.io protocol labels Dec 20, 2023
brandonsimpson21 added a commit to brandonsimpson21/socketioxide that referenced this issue Dec 23, 2023
matchit router does not allow iterating through or deleting paths only getting them.
to minimize downstream changes a new type `ClientNs` is introduced.
it holds a hashset of paths and the router itself

closes Totodore#212
brandonsimpson21 added a commit to brandonsimpson21/socketioxide that referenced this issue Dec 23, 2023
matchit router does not allow iterating through or deleting paths only getting them
path extractors and new struct `ClientNs` which holds a hashset of paths and the router itself are introduced

closes Totodore#212
@Totodore
Copy link
Owner

We need to have node removal feature to use the matchit router : ibraheemdev/matchit#44 as found by @brandonsimpson21.

@tausifcreates
Copy link
Contributor

We need to have node removal feature to use the matchit router : ibraheemdev/matchit#44 as found by @brandonsimpson21.

Do we have nodes in Socketioxide?

@brandonsimpson21
Copy link

you cannot remove nodes with the matchit router, but you could use the active nodes to recreate the router on node deletion or verify the nodes active beforehand.

@Totodore
Copy link
Owner

you cannot remove nodes with the matchit router, but you could use the active nodes to recreate the router on node deletion or verify the nodes active beforehand.

It is a solution that is quite convoluted. I'll try to propose a PR for node removal for the matchit router before thinking about alternative solutions.

@tausifcreates
Copy link
Contributor

@Totodore can we go the regex route and leave the case of engine slowing down due to bad regex upto the user 😄

@Totodore
Copy link
Owner

ibraheemdev/matchit#49

@Totodore
Copy link
Owner

ibraheemdev/matchit#49

Merged and released with matchit 0.8.2

@Totodore Totodore linked a pull request Jun 7, 2024 that will close this issue
3 tasks
@Totodore Totodore linked a pull request Jun 7, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed socketio-v4 It is only about the version 4 of the socket.io protocol socketio-v5 It is only about the version 5 of the socket.io protocol
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

4 participants