From 5d76330430757234c5c8b17a73ca7c8dd279d194 Mon Sep 17 00:00:00 2001 From: gounux Date: Tue, 8 Oct 2024 14:18:51 +0200 Subject: [PATCH 1/9] feature: handle users registration --- gischat/app.py | 121 ++++++++++++++++++++++++++++----- gischat/models.py | 7 +- gischat/templates/ws-page.html | 18 +++-- 3 files changed, 123 insertions(+), 23 deletions(-) diff --git a/gischat/app.py b/gischat/app.py index 9b0d102..c9283a7 100755 --- a/gischat/app.py +++ b/gischat/app.py @@ -14,7 +14,8 @@ from gischat import INTERNAL_MESSAGE_AUTHOR from gischat.models import ( - InternalMessageModel, + InternalNbUsersMessageModel, + InternalNewcomerMessageModel, MessageModel, RulesModel, StatusModel, @@ -43,13 +44,18 @@ class WebsocketNotifier: Class used to broadcast messages to registered websockets """ + # list of websockets connection associated to room connections: dict[str, list[WebSocket]] + # list of user nicknames associated to their websocket + users: dict[WebSocket, str] + def __init__(self): # registered websockets for rooms self.connections: dict[str, list[WebSocket]] = { room: [] for room in available_rooms() } + self.users = {} self.generator = self.get_notification_generator() async def get_notification_generator(self): @@ -67,6 +73,7 @@ async def connect(self, room: str, websocket: WebSocket) -> None: def remove(self, room: str, websocket: WebSocket) -> None: if websocket in self.connections[room]: self.connections[room].remove(websocket) + self.unregister_user(websocket) async def notify(self, room: str, message: str) -> None: living_connections = [] @@ -81,15 +88,71 @@ async def notify(self, room: str, message: str) -> None: logger.error("Can not send message to disconnected websocket") self.connections[room] = living_connections - def get_nb_users(self, room: str) -> int: + def get_nb_connected_users(self, room: str) -> int: + """ + Returns the number of connected users in a room + :param room: room to check + :return: number of connected users in a room + """ return len(self.connections[room]) - async def notify_internal(self, room: str) -> None: - message = InternalMessageModel( - author=INTERNAL_MESSAGE_AUTHOR, nb_users=self.get_nb_users(room) + async def notify_nb_users(self, room: str) -> None: + """ + Notifies connected users in a room with the number of connected users + :param room: room to notify + """ + message = InternalNbUsersMessageModel( + author=INTERNAL_MESSAGE_AUTHOR, nb_users=self.get_nb_connected_users(room) ) await self.notify(room, json.dumps(jsonable_encoder(message))) + def register_user(self, websocket: WebSocket, user: str) -> None: + """ + Registers a user assigned to a websocket + :param websocket: user's websocket + :param user: user's nickname + """ + self.users[websocket] = user + + def unregister_user(self, websocket: WebSocket) -> None: + """ + Unregister a user given with the websocket + :param websocket: user's websocket to unregister + """ + if websocket in self.users.keys(): + del self.users[websocket] + + def get_registered_users(self, room: str) -> list[str]: + """ + Returns the name of users present in a room + :param room: room to check + :return: List of user names + """ + users = [] + for ws in self.connections[room]: + # use try/except instead of list comprehension + # in case a user didn't register itself + try: + users.append(self.users[ws]) + except KeyError: + continue + return users + + def is_user_present(self, room: str, user: str) -> bool: + """ + Checks if a user given by the nickname is present in a room + :param room: room to check + :param user: user to check + :return: True if present, False otherwise + """ + for ws in self.connections[room]: + try: + if self.users[ws] == user: + return True + except KeyError: + continue + return False + notifier = WebsocketNotifier() @@ -152,6 +215,13 @@ async def get_rules() -> RulesModel: ) +@app.get("/room/{room}/users") +async def get_connected_users(room: str) -> list[str]: + if room not in notifier.connections.keys(): + raise HTTPException(status_code=404, detail=f"Room '{room}' not registered") + return sorted(notifier.get_registered_users(room), key=str.casefold) + + @app.put( "/room/{room}/message", response_model=MessageModel, @@ -172,23 +242,40 @@ async def websocket_endpoint(websocket: WebSocket, room: str) -> None: raise HTTPException(status_code=404, detail=f"Room '{room}' not registered") await notifier.connect(room, websocket) - await notifier.notify_internal(room) + await notifier.notify_nb_users(room) logger.info(f"New websocket connected in room '{room}'") try: while True: data = await websocket.receive_text() - try: - message = MessageModel(**json.loads(data)) - except ValidationError: - logger.error("Invalid message in websocket") - continue - logger.info(f"Message in room '{room}': {message}") - try: - await notifier.notify(room, json.dumps(jsonable_encoder(message))) - except WebSocketDisconnect: - logger.error("Can not send message to disconnected websocket") + payload = json.loads(data) + if "author" in payload and payload["author"] == "internal": + if "newcomer" in payload: + message = InternalNewcomerMessageModel(**payload) + newcomer = payload["newcomer"] + notifier.register_user(websocket, newcomer) + logger.info(f"Newcomer in room {room}: {newcomer}") + + try: + await notifier.notify( + room, json.dumps(jsonable_encoder(message)) + ) + except WebSocketDisconnect: + logger.error("Can not send message to disconnected websocket") + + else: + try: + message = MessageModel(**payload) + logger.info(f"Message in room '{room}': {message}") + except ValidationError: + logger.error("Invalid message in websocket") + continue + + try: + await notifier.notify(room, json.dumps(jsonable_encoder(message))) + except WebSocketDisconnect: + logger.error("Can not send message to disconnected websocket") except WebSocketDisconnect: notifier.remove(room, websocket) - await notifier.notify_internal(room) + await notifier.notify_nb_users(room) logger.info(f"Websocket disconnected from room '{room}'") diff --git a/gischat/models.py b/gischat/models.py index 67f74af..cc22b08 100755 --- a/gischat/models.py +++ b/gischat/models.py @@ -43,6 +43,11 @@ def __str__(self) -> str: return f"[{self.author}]: '{self.message}'" -class InternalMessageModel(BaseModel): +class InternalNbUsersMessageModel(BaseModel): author: str nb_users: int + + +class InternalNewcomerMessageModel(BaseModel): + author: str + newcomer: str diff --git a/gischat/templates/ws-page.html b/gischat/templates/ws-page.html index 81a3698..d2d1d60 100644 --- a/gischat/templates/ws-page.html +++ b/gischat/templates/ws-page.html @@ -30,7 +30,7 @@

gischat web client


- +

@@ -86,7 +86,6 @@

gischat web client

instance.disabled = enabled; ssl.disabled = enabled; document.getElementById("roomId").disabled = enabled; - document.getElementById("authorId").disabled = !enabled; document.getElementById("avatarId").disabled = !enabled; document.getElementById("messageText").disabled = !enabled; document.getElementById("sendButton").disabled = !enabled; @@ -112,13 +111,22 @@

gischat web client

displayMessage(`Connected to websocket in room ${room.value}`); connected = true; setFormEnabled(true); + websocket.send(JSON.stringify({author: "internal", newcomer: document.getElementById("authorId").value})); } websocket.onmessage = (event) => { const data = JSON.parse(event.data); const author = data.author; - const log = author === "internal" ? - `${data.nb_users} user${data.nb_users > 1 ? "s" : ""} connected in room` : - `[${author}] (${new Date().toLocaleTimeString()}): ${data.message}`; + let log; + if (author === "internal") { + if (data.nb_users) { + log = `${data.nb_users} user${data.nb_users > 1 ? "s" : ""} connected in room`; + } + if (data.newcomer) { + log = `${data.newcomer} has joined the room`; + } + } else { + log = `[${author}] (${new Date().toLocaleTimeString()}): ${data.message}` + } displayMessage(log); }; websocket.onerror = (error) => { From 7d07cf1e26fae5b3b29ce4586ac856ab316d209b Mon Sep 17 00:00:00 2001 From: gounux Date: Tue, 8 Oct 2024 17:40:05 +0200 Subject: [PATCH 2/9] refactor: refactor newcomer notification --- gischat/app.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/gischat/app.py b/gischat/app.py index c9283a7..74a001f 100755 --- a/gischat/app.py +++ b/gischat/app.py @@ -106,6 +106,17 @@ async def notify_nb_users(self, room: str) -> None: ) await self.notify(room, json.dumps(jsonable_encoder(message))) + async def notify_newcomer(self, room: str, user: str) -> None: + """ + Notifies a room that a newcomer has joined + :param room: room to notify + :param user: nickname of the newcomer + """ + message = InternalNewcomerMessageModel( + author=INTERNAL_MESSAGE_AUTHOR, newcomer=user + ) + await self.notify(room, json.dumps(jsonable_encoder(message))) + def register_user(self, websocket: WebSocket, user: str) -> None: """ Registers a user assigned to a websocket @@ -116,7 +127,7 @@ def register_user(self, websocket: WebSocket, user: str) -> None: def unregister_user(self, websocket: WebSocket) -> None: """ - Unregister a user given with the websocket + Unregisters a user given with the websocket :param websocket: user's websocket to unregister """ if websocket in self.users.keys(): @@ -124,7 +135,7 @@ def unregister_user(self, websocket: WebSocket) -> None: def get_registered_users(self, room: str) -> list[str]: """ - Returns the name of users present in a room + Returns the nicknames of users registered in a room :param room: room to check :return: List of user names """ @@ -251,17 +262,10 @@ async def websocket_endpoint(websocket: WebSocket, room: str) -> None: payload = json.loads(data) if "author" in payload and payload["author"] == "internal": if "newcomer" in payload: - message = InternalNewcomerMessageModel(**payload) newcomer = payload["newcomer"] notifier.register_user(websocket, newcomer) logger.info(f"Newcomer in room {room}: {newcomer}") - - try: - await notifier.notify( - room, json.dumps(jsonable_encoder(message)) - ) - except WebSocketDisconnect: - logger.error("Can not send message to disconnected websocket") + await notifier.notify_newcomer(room, newcomer) else: try: @@ -270,11 +274,8 @@ async def websocket_endpoint(websocket: WebSocket, room: str) -> None: except ValidationError: logger.error("Invalid message in websocket") continue + await notifier.notify(room, json.dumps(jsonable_encoder(message))) - try: - await notifier.notify(room, json.dumps(jsonable_encoder(message))) - except WebSocketDisconnect: - logger.error("Can not send message to disconnected websocket") except WebSocketDisconnect: notifier.remove(room, websocket) await notifier.notify_nb_users(room) From 99aa64cce89e83d1c8948aa593d1c536d45a99f8 Mon Sep 17 00:00:00 2001 From: gounux Date: Tue, 8 Oct 2024 17:46:12 +0200 Subject: [PATCH 3/9] tests: add users route test --- tests/test_api.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_api.py b/tests/test_api.py index 5f800d3..ab8f8c2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -43,6 +43,13 @@ def test_get_rules(client: TestClient): assert response.json()["max_message_length"] == int(MAX_MESSAGE_LENGTH) +@pytest.mark.parametrize("room", test_rooms()) +def test_get_users(client: TestClient, room: str): + response = client.get(f"/room/{room}/users") + assert response.status_code == 200 + assert response.json() == [] + + @pytest.mark.parametrize("room", test_rooms()) def test_put_message(client: TestClient, room: str): response = client.put( From ce588e0a0c483a7fadce4330a3ed270d7bb0180c Mon Sep 17 00:00:00 2001 From: gounux Date: Tue, 8 Oct 2024 17:55:06 +0200 Subject: [PATCH 4/9] tests: add newcomer registration tests --- tests/test_websocket.py | 60 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/test_websocket.py b/tests/test_websocket.py index 9cda1a2..a0b2c8b 100644 --- a/tests/test_websocket.py +++ b/tests/test_websocket.py @@ -103,3 +103,63 @@ def test_websocket_uncompliant_message(client: TestClient, room: str): "author": INTERNAL_MESSAGE_AUTHOR, "nb_users": 2, } + + +@pytest.mark.parametrize("room", test_rooms()) +def test_websocket_send_newcomer(client: TestClient, room: str): + with client.websocket_connect(f"/room/{room}/ws") as websocket: + websocket.send_json({"author": INTERNAL_MESSAGE_AUTHOR, "newcomer": "Isidore"}) + assert websocket.receive_json() == { + "author": INTERNAL_MESSAGE_AUTHOR, + "nb_users": 1, + } + assert websocket.receive_json() == { + "author": INTERNAL_MESSAGE_AUTHOR, + "newcomer": "Isidore", + } + + +@pytest.mark.parametrize("room", test_rooms()) +def test_websocket_send_newcomer_multiple(client: TestClient, room: str): + with client.websocket_connect(f"/room/{room}/ws") as websocket1: + websocket1.send_json({"author": INTERNAL_MESSAGE_AUTHOR, "newcomer": "user1"}) + with client.websocket_connect(f"/room/{room}/ws") as websocket2: + websocket2.send_json( + {"author": INTERNAL_MESSAGE_AUTHOR, "newcomer": "user2"} + ) + assert websocket1.receive_json() == { + "author": INTERNAL_MESSAGE_AUTHOR, + "nb_users": 1, + } + assert websocket1.receive_json() == { + "author": INTERNAL_MESSAGE_AUTHOR, + "newcomer": "user1", + } + assert websocket1.receive_json() == { + "author": INTERNAL_MESSAGE_AUTHOR, + "nb_users": 2, + } + assert websocket1.receive_json() == { + "author": INTERNAL_MESSAGE_AUTHOR, + "newcomer": "user2", + } + assert websocket2.receive_json() == { + "author": INTERNAL_MESSAGE_AUTHOR, + "nb_users": 2, + } + assert websocket2.receive_json() == { + "author": INTERNAL_MESSAGE_AUTHOR, + "newcomer": "user2", + } + + +@pytest.mark.parametrize("room", test_rooms()) +def test_websocket_send_newcomer_api_call(client: TestClient, room: str): + with client.websocket_connect(f"/room/{room}/ws") as websocket1: + websocket1.send_json({"author": INTERNAL_MESSAGE_AUTHOR, "newcomer": "Isidore"}) + assert client.get(f"/room/{room}/users").json() == ["Isidore"] + with client.websocket_connect(f"/room/{room}/ws") as websocket2: + websocket2.send_json( + {"author": INTERNAL_MESSAGE_AUTHOR, "newcomer": "Barnabe"} + ) + assert client.get(f"/room/{room}/users").json() == ["Barnabe", "Isidore"] From 56597b6fe40fac9f2bd878f99b73d0d3afc4b2d1 Mon Sep 17 00:00:00 2001 From: gounux Date: Tue, 8 Oct 2024 17:57:25 +0200 Subject: [PATCH 5/9] fix: set newcomer field validators --- gischat/models.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/gischat/models.py b/gischat/models.py index cc22b08..32c7e3e 100755 --- a/gischat/models.py +++ b/gischat/models.py @@ -50,4 +50,9 @@ class InternalNbUsersMessageModel(BaseModel): class InternalNewcomerMessageModel(BaseModel): author: str - newcomer: str + newcomer: str = Field( + None, + min_length=int(os.environ.get("MIN_AUTHOR_LENGTH", 3)), + max_length=int(os.environ.get("MAX_AUTHOR_LENGTH", 32)), + pattern=r"^[a-z-A-Z-0-9-_]+$", + ) From ffa606e7e79f2daa28a989c62c5448b2b6d31c8e Mon Sep 17 00:00:00 2001 From: gounux Date: Tue, 8 Oct 2024 20:33:59 +0200 Subject: [PATCH 6/9] feature: notify room when a user has left --- gischat/app.py | 31 ++++++++++++++++++++----------- gischat/models.py | 10 ++++++++++ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/gischat/app.py b/gischat/app.py index 74a001f..e784d2a 100755 --- a/gischat/app.py +++ b/gischat/app.py @@ -14,6 +14,7 @@ from gischat import INTERNAL_MESSAGE_AUTHOR from gischat.models import ( + InternalExiterMessageModel, InternalNbUsersMessageModel, InternalNewcomerMessageModel, MessageModel, @@ -70,10 +71,15 @@ async def connect(self, room: str, websocket: WebSocket) -> None: await websocket.accept() self.connections[room].append(websocket) - def remove(self, room: str, websocket: WebSocket) -> None: + async def remove(self, room: str, websocket: WebSocket) -> None: + # remove websocket from connections if websocket in self.connections[room]: self.connections[room].remove(websocket) - self.unregister_user(websocket) + # unregister user + if websocket in self.users.keys(): + exiter = self.users[websocket] + del self.users[websocket] + await self.notify_exiter(room, exiter) async def notify(self, room: str, message: str) -> None: living_connections = [] @@ -117,6 +123,17 @@ async def notify_newcomer(self, room: str, user: str) -> None: ) await self.notify(room, json.dumps(jsonable_encoder(message))) + async def notify_exiter(self, room: str, user: str) -> None: + """ + Notifies a room that a user has left the room + :param room: room to notify + :param user: nickname of the exiter + """ + message = InternalExiterMessageModel( + author=INTERNAL_MESSAGE_AUTHOR, exiter=user + ) + await self.notify(room, json.dumps(jsonable_encoder(message))) + def register_user(self, websocket: WebSocket, user: str) -> None: """ Registers a user assigned to a websocket @@ -125,14 +142,6 @@ def register_user(self, websocket: WebSocket, user: str) -> None: """ self.users[websocket] = user - def unregister_user(self, websocket: WebSocket) -> None: - """ - Unregisters a user given with the websocket - :param websocket: user's websocket to unregister - """ - if websocket in self.users.keys(): - del self.users[websocket] - def get_registered_users(self, room: str) -> list[str]: """ Returns the nicknames of users registered in a room @@ -277,6 +286,6 @@ async def websocket_endpoint(websocket: WebSocket, room: str) -> None: await notifier.notify(room, json.dumps(jsonable_encoder(message))) except WebSocketDisconnect: - notifier.remove(room, websocket) + await notifier.remove(room, websocket) await notifier.notify_nb_users(room) logger.info(f"Websocket disconnected from room '{room}'") diff --git a/gischat/models.py b/gischat/models.py index 32c7e3e..ed7a088 100755 --- a/gischat/models.py +++ b/gischat/models.py @@ -56,3 +56,13 @@ class InternalNewcomerMessageModel(BaseModel): max_length=int(os.environ.get("MAX_AUTHOR_LENGTH", 32)), pattern=r"^[a-z-A-Z-0-9-_]+$", ) + + +class InternalExiterMessageModel(BaseModel): + author: str + exiter: str = Field( + None, + min_length=int(os.environ.get("MIN_AUTHOR_LENGTH", 3)), + max_length=int(os.environ.get("MAX_AUTHOR_LENGTH", 32)), + pattern=r"^[a-z-A-Z-0-9-_]+$", + ) From 529560dc1d13b5888219287f92d7f299afd9ea9d Mon Sep 17 00:00:00 2001 From: gounux Date: Tue, 8 Oct 2024 20:39:18 +0200 Subject: [PATCH 7/9] feature: notify room when a user has left in web page --- gischat/templates/ws-page.html | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gischat/templates/ws-page.html b/gischat/templates/ws-page.html index d2d1d60..42114fb 100644 --- a/gischat/templates/ws-page.html +++ b/gischat/templates/ws-page.html @@ -124,6 +124,9 @@

gischat web client

if (data.newcomer) { log = `${data.newcomer} has joined the room`; } + if (data.exiter) { + log = `${data.exiter} has left the room`; + } } else { log = `[${author}] (${new Date().toLocaleTimeString()}): ${data.message}` } From 2c076cae6260070f2c824358e4311e9097cc866e Mon Sep 17 00:00:00 2001 From: gounux Date: Wed, 9 Oct 2024 08:16:40 +0200 Subject: [PATCH 8/9] doc: update README.md with new message types --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index bddbe3a..e7b2b8f 100644 --- a/README.md +++ b/README.md @@ -41,9 +41,13 @@ Following instances are up and running : - Number of connected users can be fetched using [the `/status` endpoint](https://gischat.geotribu.net/docs#/default/get_status_status_get) - New users must connect a websocket to the `/room/{room_name}/ws` endpoint - Messages passing through the websocket are simple JSON dicts like this: `{"message": "hello", "author": "Hans Hibbel", "avatar": "mGeoPackage.svg"}` -- :warning: Messages having the `"internal"` author are internal messages and should not be printed, they contain technical information: `{"author": "internal", "nb_users": 36}` +- :warning: Messages having the `"internal"` author are internal messages and should not be printed, they contain technical information: + - `{"author": "internal", "nb_users": 36}` -> there are now 36 users in the room + - `{"author": "internal", "newcomer": "Jane"}` -> Jane has joined the room + - `{"author": "internal", "exiter": "Jane"}` -> Jane has left the room - `"author"` value must be alphanumeric (or `_` or `-`) and have min / max length set by `MIN_AUTHOR_LENGTH` / `MAX_AUTHOR_LENGTH` environment variables - `"message"` value must have max length set by `MAX_MESSAGE_LENGTH` environment variable +- Once the websocket is connected, it might be polite to send a registration message like : `{"author": "internal", "newcomer": "Jane"}` ## Deploy a self-hosted instance From 567f5553511a0bca9311a0cd5dc05a6f88298467 Mon Sep 17 00:00:00 2001 From: gounux Date: Sun, 13 Oct 2024 10:43:16 +0200 Subject: [PATCH 9/9] doc: add docstrings --- gischat/app.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/gischat/app.py b/gischat/app.py index e784d2a..099a3c3 100755 --- a/gischat/app.py +++ b/gischat/app.py @@ -37,6 +37,10 @@ def available_rooms() -> list[str]: + """ + Returns list of available rooms + :return: list of available rooms + """ return os.environ.get("ROOMS", "QGIS,QField,Geotribu").split(",") @@ -68,10 +72,21 @@ async def push(self, msg: str) -> None: await self.generator.asend(msg) async def connect(self, room: str, websocket: WebSocket) -> None: + """ + Connects a new user to a room + :param room: room to connect the websocket to + :param websocket: new user's websocket connection + """ await websocket.accept() self.connections[room].append(websocket) async def remove(self, room: str, websocket: WebSocket) -> None: + """ + Removes a user from a room + Should be called when a websocket is disconnected + :param room: room to disconnect user from + :param websocket: user's websocket connection + """ # remove websocket from connections if websocket in self.connections[room]: self.connections[room].remove(websocket) @@ -82,6 +97,11 @@ async def remove(self, room: str, websocket: WebSocket) -> None: await self.notify_exiter(room, exiter) async def notify(self, room: str, message: str) -> None: + """ + Sends a message to a room + :param room: room to notify + :param message: message to send, should be stringified JSON + """ living_connections = [] while len(self.connections[room]) > 0: # Looping like this is necessary in case a disconnection is handled @@ -160,7 +180,7 @@ def get_registered_users(self, room: str) -> list[str]: def is_user_present(self, room: str, user: str) -> bool: """ - Checks if a user given by the nickname is present in a room + Checks if a user given by the nickname is registered in a room :param room: room to check :param user: user to check :return: True if present, False otherwise @@ -177,6 +197,7 @@ def is_user_present(self, room: str, user: str) -> bool: notifier = WebsocketNotifier() +# initialize sentry if "SENTRY_DSN" in os.environ and os.environ.get("SENTRY_DSN"): sentry_sdk.init( dsn=os.environ.get("SENTRY_DSN"), @@ -188,7 +209,7 @@ def is_user_present(self, room: str, user: str) -> bool: app = FastAPI( title="gischat API", - summary="Chat with your GIS tribe in QGIS, QField and other clients !", + summary="Chat with your GIS tribe in QGIS, GIS mobile apps and other clients !", version=get_poetry_version(), ) templates = Jinja2Templates(directory="gischat/templates")