MultiServer, customserver, CI, Test: Fix problems in room hosting and test/simulate it

.github/workflows/unittests.yml

@@ -24,7 +24,7 @@ on:
       - '.github/workflows/unittests.yml'
 
 jobs:
-  build:
+  unit:
     runs-on: ${{ matrix.os }}
     name: Test Python ${{ matrix.python.version }} ${{ matrix.os }}
 
@@ -60,3 +60,32 @@ jobs:
     - name: Unittests
       run: |
         pytest -n auto
+
+  hosting:
+    runs-on: ${{ matrix.os }}
+    name: Test hosting with ${{ matrix.python.version }} on ${{ matrix.os }}
+
+    strategy:
+      matrix:
+        os:
+          - ubuntu-latest
+        python:
+          - {version: '3.11'}  # current
+
+    steps:
+    - uses: actions/checkout@v4
+    - name: Set up Python ${{ matrix.python.version }}
+      uses: actions/setup-python@v5
+      with:
+        python-version: ${{ matrix.python.version }}
+    - name: Install dependencies
+      run: |
+        python -m venv venv
+        source venv/bin/activate
+        python -m pip install --upgrade pip
+        python ModuleUpdate.py --yes --force --append "WebHostLib/requirements.txt"
+    - name: Test hosting
+      run: |
+        source venv/bin/activate
+        export PYTHONPATH=$(pwd)
+        python test/hosting/__main__.py

MultiServer.py

@@ -3,6 +3,7 @@
 import argparse
 import asyncio
 import collections
+import contextlib
 import copy
 import datetime
 import functools
@@ -176,7 +177,7 @@ class Context:
     location_name_groups: typing.Dict[str, typing.Dict[str, typing.Set[str]]]
     all_item_and_group_names: typing.Dict[str, typing.Set[str]]
     all_location_and_group_names: typing.Dict[str, typing.Set[str]]
-    non_hintable_names: typing.Dict[str, typing.Set[str]]
+    non_hintable_names: typing.Dict[str, typing.AbstractSet[str]]
     spheres: typing.List[typing.Dict[int, typing.Set[int]]]
     """ each sphere is { player: { location_id, ... } } """
     logger: logging.Logger
@@ -231,7 +232,7 @@ def __init__(self, host: str, port: int, server_password: str, password: str, lo
         self.embedded_blacklist = {"host", "port"}
         self.client_ids: typing.Dict[typing.Tuple[int, int], datetime.datetime] = {}
         self.auto_save_interval = 60  # in seconds
-        self.auto_saver_thread = None
+        self.auto_saver_thread: typing.Optional[threading.Thread] = None
         self.save_dirty = False
         self.tags = ['AP']
         self.games: typing.Dict[int, str] = {}
@@ -268,6 +269,11 @@ def _load_game_data(self):
         for world_name, world in worlds.AutoWorldRegister.world_types.items():
             self.non_hintable_names[world_name] = world.hint_blacklist
 
+        for game_package in self.gamespackage.values():
+            # remove groups from data sent to clients
+            del game_package["item_name_groups"]
+            del game_package["location_name_groups"]
+
     def _init_game_data(self):
         for game_name, game_package in self.gamespackage.items():
             if "checksum" in game_package:
@@ -1926,8 +1932,6 @@ def _cmd_status(self, tag: str = "") -> bool:
     def _cmd_exit(self) -> bool:
         """Shutdown the server"""
         self.ctx.server.ws_server.close()
-        if self.ctx.shutdown_task:
-            self.ctx.shutdown_task.cancel()
         self.ctx.exit_event.set()
         return True
 
@@ -2285,7 +2289,8 @@ def parse_args() -> argparse.Namespace:
 
 
 async def auto_shutdown(ctx, to_cancel=None):
-    await asyncio.sleep(ctx.auto_shutdown)
+    with contextlib.suppress(asyncio.TimeoutError):
+        await asyncio.wait_for(ctx.exit_event.wait(), ctx.auto_shutdown)
 
     def inactivity_shutdown():
         ctx.server.ws_server.close()
@@ -2305,7 +2310,8 @@ def inactivity_shutdown():
             if seconds < 0:
                 inactivity_shutdown()
             else:
-                await asyncio.sleep(seconds)
+                with contextlib.suppress(asyncio.TimeoutError):
+                    await asyncio.wait_for(ctx.exit_event.wait(), seconds)
 
 
 def load_server_cert(path: str, cert_key: typing.Optional[str]) -> "ssl.SSLContext":

WebHost.py

@@ -12,14 +12,17 @@
 import Utils
 import settings
 
+if typing.TYPE_CHECKING:
+    from flask import Flask
+
 Utils.local_path.cached_path = os.path.dirname(__file__) or "."  # py3.8 is not abs. remove "." when dropping 3.8
 settings.no_gui = True
 configpath = os.path.abspath("config.yaml")
 if not os.path.exists(configpath):  # fall back to config.yaml in home
     configpath = os.path.abspath(Utils.user_path('config.yaml'))
 
 
-def get_app():
+def get_app() -> "Flask":
     from WebHostLib import register, cache, app as raw_app
     from WebHostLib.models import db
 

WebHostLib/customserver.py

@@ -168,17 +168,28 @@ def get_random_port():
 def get_static_server_data() -> dict:
     import worlds
     data = {
-        "non_hintable_names": {},
-        "gamespackage": worlds.network_data_package["games"],
-        "item_name_groups": {world_name: world.item_name_groups for world_name, world in
-                             worlds.AutoWorldRegister.world_types.items()},
-        "location_name_groups": {world_name: world.location_name_groups for world_name, world in
-                                 worlds.AutoWorldRegister.world_types.items()},
+        "non_hintable_names": {
+            world_name: world.hint_blacklist
+            for world_name, world in worlds.AutoWorldRegister.world_types.items()
+        },
+        "gamespackage": {
+            world_name: {
+                key: value
+                for key, value in game_package.items()
+                if key not in ("item_name_groups", "location_name_groups")
+            }
+            for world_name, game_package in worlds.network_data_package["games"].items()
+        },
+        "item_name_groups": {
+            world_name: world.item_name_groups
+            for world_name, world in worlds.AutoWorldRegister.world_types.items()
+        },
+        "location_name_groups": {
+            world_name: world.location_name_groups
+            for world_name, world in worlds.AutoWorldRegister.world_types.items()
+        },
     }
 
-    for world_name, world in worlds.AutoWorldRegister.world_types.items():
-        data["non_hintable_names"][world_name] = world.hint_blacklist
-
     return data
 
 
@@ -283,6 +294,9 @@ async def start_room(room_id):
                     ctx._save()
             finally:
                 try:
+                    ctx.save_dirty = False  # make sure the saving thread does not write to DB after final wakeup
+                    ctx.exit_event.set()  # make sure the saving thread stops at some point
+                    # NOTE: async saving should probably be an async task and could be merged with shutdown_task
                     with (db_session):
                         # ensure the Room does not spin up again on its own, minute of safety buffer
                         room = Room.get(id=room_id)
@@ -294,10 +308,22 @@ async def start_room(room_id):
                     rooms_shutting_down.put(room_id)
 
     class Starter(threading.Thread):
+        _tasks: typing.List[asyncio.Future]
+
+        def __init__(self):
+            super().__init__()
+            self._tasks = []
+
+        def _done(self, task: asyncio.Future):
+            self._tasks.remove(task)
+            task.result()
+
         def run(self):
             while 1:
                 next_room = rooms_to_run.get(block=True,  timeout=None)
-                asyncio.run_coroutine_threadsafe(start_room(next_room), loop)
+                task = asyncio.run_coroutine_threadsafe(start_room(next_room), loop)
+                self._tasks.append(task)
+                task.add_done_callback(self._done)
                 logging.info(f"Starting room {next_room} on {name}.")
 
     starter = Starter()

test/hosting/__main__.py

@@ -0,0 +1,180 @@
+# A bunch of tests to verify MultiServer and custom webhost server work as expected.
+# This spawns processes and may modify your local AP, so this is not run as part of unit testing.
+# Run with `python test/hosting` instead,
+import logging
+import traceback
+from tempfile import TemporaryDirectory
+from time import sleep
+from typing import Any
+
+from test.hosting.client import Client
+from test.hosting.generate import generate_local
+from test.hosting.serve import ServeGame, LocalServeGame, WebHostServeGame
+from test.hosting.webhost import (create_room, get_app, get_multidata_for_room, set_multidata_for_room, start_room,
+                                  stop_autohost, upload_multidata)
+from test.hosting.world import copy as copy_world, delete as delete_world
+
+failure = False
+fail_fast = True
+
+
+def assert_true(condition: Any, msg: str = "") -> None:
+    global failure
+    if not condition:
+        failure = True
+        msg = f": {msg}" if msg else ""
+        raise AssertionError(f"Assertion failed{msg}")
+
+
+def assert_equal(first: Any, second: Any, msg: str = "") -> None:
+    global failure
+    if first != second:
+        failure = True
+        msg = f": {msg}" if msg else ""
+        raise AssertionError(f"Assertion failed: {first} == {second}{msg}")
+
+
+if fail_fast:
+    expect_true = assert_true
+    expect_equal = assert_equal
+else:
+    def expect_true(condition: Any, msg: str = "") -> None:
+        global failure
+        if not condition:
+            failure = True
+            tb = "".join(traceback.format_stack()[:-1])
+            msg = f": {msg}" if msg else ""
+            logging.error(f"Expectation failed{msg}\n{tb}")
+
+    def expect_equal(first: Any, second: Any, msg: str = "") -> None:
+        global failure
+        if first != second:
+            failure = True
+            tb = "".join(traceback.format_stack()[:-1])
+            msg = f": {msg}" if msg else ""
+            logging.error(f"Expectation failed {first} == {second}{msg}\n{tb}")
+
+
+if __name__ == "__main__":
+    import warnings
+    warnings.simplefilter("ignore", ResourceWarning)
+    warnings.simplefilter("ignore", UserWarning)
+
+    spacer = '=' * 80
+
+    with TemporaryDirectory() as tempdir:
+        multis = [["Clique"], ["Temp World"], ["Clique", "Temp World"]]
+        p1_games = []
+        data_paths = []
+        rooms = []
+
+        copy_world("Clique", "Temp World")
+        try:
+            for n, games in enumerate(multis, 1):
+                print(f"Generating [{n}] {', '.join(games)}")
+                multidata = generate_local(games, tempdir)
+                print(f"Generated [{n}] {', '.join(games)} as {multidata}\n")
+                p1_games.append(games[0])
+                data_paths.append(multidata)
+        finally:
+            delete_world("Temp World")
+
+        webapp = get_app(tempdir)
+        webhost_client = webapp.test_client()
+        for n, multidata in enumerate(data_paths, 1):
+            seed = upload_multidata(webhost_client, multidata)
+            room = create_room(webhost_client, seed)
+            print(f"Uploaded [{n}] {multidata} as {room}\n")
+            rooms.append(room)
+
+        print("Starting autohost")
+        from WebHostLib.autolauncher import autohost
+        try:
+            autohost(webapp.config)
+
+            host: ServeGame
+            for n, (multidata, room, game, multi_games) in enumerate(zip(data_paths, rooms, p1_games, multis), 1):
+                involved_games = {"Archipelago"} | set(multi_games)
+                for collected_items in (0, 1):
+                    print(f"\nTesting [{n}] {game} in {multidata} on MultiServer with {collected_items} items collected")
+                    with LocalServeGame(multidata) as host:
+                        with Client(host.address, game, "Player1") as client:
+                            local_data_packages = client.games_packages
+                            local_collected_items = len(client.checked_locations)
+                            client.collect_any()
+
+                    for game_name in sorted(involved_games):
+                        expect_true(game_name in local_data_packages,
+                                    f"{game_name} missing from MultiServer datap ackage")
+                        expect_true("item_name_groups" not in local_data_packages.get(game_name, {}),
+                                    f"item_name_groups are not supposed to be in MultiServer data for {game_name}")
+                        expect_true("location_name_groups" not in local_data_packages.get(game_name, {}),
+                                    f"location_name_groups are not supposed to be in MultiServer data for {game_name}")
+                    for game_name in local_data_packages:
+                        expect_true(game_name in involved_games,
+                                    f"Received unexpected extra data package for {game_name} from MultiServer")
+                    assert_equal(local_collected_items, collected_items,
+                                 "MultiServer did not load or save correctly")
+
+                    print(f"\nTesting [{n}] {game} in {multidata} on customserver with {collected_items} items collected")
+                    prev_host_adr: str
+                    with WebHostServeGame(webhost_client, room) as host:
+                        prev_host_adr = host.address
+                        with Client(host.address, game, "Player1") as client:
+                            web_data_packages = client.games_packages
+                            web_collected_items = len(client.checked_locations)
+                            client.collect_any()
+
+                    # verify server shut down
+                    try:
+                        with Client(prev_host_adr, game, "Player1") as client:
+                            assert_true(False, "Server did not shut down")
+                    except ConnectionError:
+                        pass
+
+                    for game_name in sorted(involved_games):
+                        expect_true(game_name in web_data_packages,
+                                    f"{game_name} missing from customserver data package")
+                        expect_true("item_name_groups" not in web_data_packages.get(game_name, {}),
+                                    f"item_name_groups are not supposed to be in customserver data for {game_name}")
+                        expect_true("location_name_groups" not in web_data_packages.get(game_name, {}),
+                                    f"location_name_groups are not supposed to be in customserver data for {game_name}")
+                    for game_name in web_data_packages:
+                        expect_true(game_name in involved_games,
+                                    f"Received unexpected extra data package for {game_name} from customserver")
+                    assert_equal(web_collected_items, collected_items,
+                                 "customserver did not load or save correctly")
+
+                    # compare customserver to MultiServer
+                    expect_equal(local_data_packages, web_data_packages,
+                                 "customserver datapackage differs from MultiServer")
+
+            sleep(5.5)  # make sure all tasks actually stopped
+
+            # raise an exception in customserver and verify the save doesn't get destroyed
+            # local variables room is the last room's id here
+            old_data = get_multidata_for_room(webhost_client, room)
+            print(f"Destroying multidata for {room}")
+            set_multidata_for_room(webhost_client, room, bytes([0]))
+            try:
+                start_room(webhost_client, room, timeout=7)
+            except TimeoutError:
+                pass
+            else:
+                assert_true(False, "Room started with destroyed multidata")
+            print(f"Restoring multidata for {room}")
+            set_multidata_for_room(webhost_client, room, old_data)
+            with WebHostServeGame(webhost_client, room) as host:
+                with Client(host.address, game, "Player1") as client:
+                    assert_equal(len(client.checked_locations), 2,
+                                 "Save was destroyed during exception in customserver")
+                    print("Save file is not busted 🥳")
+
+        finally:
+            print("Stopping autohost")
+            stop_autohost(False)
+
+    if failure:
+        print("Some tests failed")
+        exit(1)
+    exit(0)