Pithikos / python-websocket-server

A simple fully working websocket-server in Python with no external dependencies
MIT License
1.14k stars 381 forks source link

Fixes accessing clients across websocket instances using send_message_to_all #65

Closed kdschlosser closed 3 years ago

kdschlosser commented 5 years ago

This corrects the issue with the send_message_to_all method sending messages to clients that are not from the websocket instance the method was called from.

I am not sure as to why it was designed this way. But to keep the ability to send the messages I made the send_message_to_all a class method that gets overridden by a class instance (only for that instance).

if you wanted to send a message to all clients on all instances you would use WebsocketServer.send_message_to_all. and instance.send_message_to_all will send to only the clients that are connected to the instance that was used in the call.

I also mangled the class level devices container and added a devices property. this way if the devices container is accessed from an instance only the clients specific to that instance are returned.

Pithikos commented 3 years ago

@kdschlosser I know this is a pretty late reply, but I didn't deal with this library for a loooong time. In any case, the reason the server works the way it does, is since it wasn't meant to have multiple server instances when I created it - so for simplicity.

I reviewd your PR now, and although you're right about how the behaviour should be, I think it can be implemented in a much cleaner way. Simply moving clients into the __init__ should fix everything without all this code. Feel free to let me know if there's something I missed.

Pithikos commented 3 years ago

This has now been fixed with version 0.5.1

kdschlosser commented 3 years ago

I didn't know if there was a specific use case for wanting to send to all clients of all web socket connections. I wanted to leave the ability to send to all clients on all connections on the off chance someone was using it or needed it. It would be available as a class method. an instance would override that class method so the same function name would be used on the instance except it would return the clients attached to that websocket connection.

The use of the __clients was so it wouldn't get accessed easily. I created a clients property that would return the clients for that instance but I still needed to have a class level container that housed all clients in order to the send to all clients class method to work.

I could have done about using a meta class o accomplish the same task. but there was no need to go that route when it could be done without it.