-
Notifications
You must be signed in to change notification settings - Fork 27
feat(server): add appstate.py, the router seam (R3) #833
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| """Shared application state — the seam that lets route modules reach core | ||
| singletons without importing ``server``. | ||
|
|
||
| ``server.py`` is the host: it owns the FastAPI ``app``, constructs the DB | ||
| singletons, and runs the lifecycle. As routes move out into ``routers/`` (R3), | ||
| those modules need ``meta_db`` and friends — but they must not ``import | ||
| server``, or the import graph goes circular the moment ``server`` imports them | ||
| back. | ||
|
|
||
| So ``server`` **injects** its singletons here once, at the point it builds them:: | ||
|
|
||
| # server.py | ||
| meta_db = MetadataDB(CONFIG_DIR) | ||
| appstate.configure(meta_db=meta_db, ...) | ||
|
|
||
| and a router reads them back as **module attributes, at call time**:: | ||
|
|
||
| # routers/artists.py | ||
| import appstate | ||
|
|
||
| @router.get("/api/artist/{name}/page") | ||
| def artist_page(name): | ||
| return appstate.meta_db.artist_page(name) | ||
|
|
||
| This is the Python analogue of the injected `configureX({...})` seams the | ||
| frontend refactor uses (stems' ``configureStreaming``, studio's | ||
| ``configureAudioGraph``, the editor's ``src/host.js``), and of the plugin | ||
| ``setup(app, context)`` contract in Principle III: dependencies flow one way, | ||
| ``server -> routers -> appstate``, and nothing imports back up. | ||
|
|
||
| Two properties this shape buys, both load-bearing: | ||
|
|
||
| * **``import appstate`` performs no IO and constructs nothing.** ``server`` | ||
| still owns construction, so the ~49 test fixtures that do | ||
| ``sys.modules.pop("server")`` + re-import (to rebuild ``meta_db`` under a | ||
| patched ``CONFIG_DIR``) keep working untouched — a singleton *owned* here | ||
| would survive that pop and go stale. | ||
| * **Reads are late-bound.** Routers must use ``appstate.meta_db``, never | ||
| ``from appstate import meta_db`` — a ``from`` import freezes the binding at | ||
| its current value, so a later ``configure()`` (or a | ||
| ``monkeypatch.setattr(appstate, "meta_db", fake)``) would not reach the | ||
| router. This is the same read-only-binding trap as ES ``import``. | ||
|
|
||
| Defaults are ``None`` on purpose: they are inert but *type-honest*, so a router | ||
| that runs before ``configure()`` fails loudly on ``NoneType`` instead of | ||
| quietly operating on a stand-in. | ||
|
|
||
| Slots are added here only when a router actually needs one — this is a seam, | ||
| not a grab-bag for everything in ``server.py``. | ||
| """ | ||
|
|
||
| # The singletons routers may read. Every name here must also be a `_SLOTS` key. | ||
| meta_db = None | ||
| audio_effect_mappings = None | ||
|
|
||
| # Declared up front so `configure()` can reject a typo'd or stale keyword | ||
| # instead of silently creating a new global that nothing ever reads. A seam | ||
| # whose wiring can no-op undetected is worse than no seam. | ||
| _SLOTS = frozenset({"meta_db", "audio_effect_mappings"}) | ||
|
|
||
|
|
||
| def configure(**kwargs) -> None: | ||
| """Publish `server`'s singletons into this module. Called once per | ||
| `server` import (and again on re-import), so it must be idempotent.""" | ||
| unknown = set(kwargs) - _SLOTS | ||
| if unknown: | ||
| raise TypeError( | ||
| f"appstate.configure() got unknown slot(s): {sorted(unknown)}. " | ||
| f"Known slots: {sorted(_SLOTS)}. Add the name to _SLOTS if a router " | ||
| f"genuinely needs it." | ||
| ) | ||
| globals().update(kwargs) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,153 @@ | ||
| """The router seam (`appstate.py`). | ||
|
|
||
| The load-bearing assertion here is `test_server_wires_the_seam`: that `server` | ||
| actually calls `appstate.configure(...)`. Every other test in this file would | ||
| pass just fine against a seam nothing ever wires up — the same class of silent | ||
| no-op that bit the frontend refactor twice when a scripted `setHostHooks` edit | ||
| stopped matching its anchor. Unit tests cannot see wiring unless you make them | ||
| look at it. | ||
| """ | ||
|
|
||
| import importlib | ||
| import sys | ||
|
|
||
| import pytest | ||
|
|
||
| import appstate | ||
|
|
||
|
|
||
| def _close_server_dbs(mod): | ||
| conn = getattr(getattr(mod, "meta_db", None), "conn", None) | ||
| if conn is not None: | ||
| getattr(mod, "_join_background_db_threads", lambda: None)() | ||
| conn.close() | ||
| ae_conn = getattr(getattr(mod, "audio_effect_mappings", None), "conn", None) | ||
| if ae_conn is not None: | ||
| ae_conn.close() | ||
|
|
||
|
|
||
| @pytest.fixture() | ||
| def isolated_server(tmp_path, monkeypatch): | ||
| """A freshly imported `server` bound to a throwaway CONFIG_DIR. | ||
|
|
||
| Importing `server` constructs `MetadataDB` + `AudioEffectsMappingDB` at | ||
| module level, so it MUST be re-imported under a patched CONFIG_DIR — an | ||
| unguarded `import server` would create/mutate the developer's real | ||
| `~/.local/share/feedback` databases. Same idiom as the other ~49 | ||
| server-importing suites. | ||
|
|
||
| Teardown restores the appstate slots as well as closing the connections: | ||
| leaving `appstate.meta_db` published but pointing at a closed sqlite handle | ||
| would hand a later test (or router) a live-looking, dead singleton. | ||
| """ | ||
| previous = (appstate.meta_db, appstate.audio_effect_mappings) | ||
| monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) | ||
| sys.modules.pop("server", None) | ||
| mod = importlib.import_module("server") | ||
| yield mod | ||
| _close_server_dbs(mod) | ||
| # Leave no half-torn-down `server` behind: the next fixture re-imports it. | ||
| sys.modules.pop("server", None) | ||
| appstate.configure(meta_db=previous[0], audio_effect_mappings=previous[1]) | ||
|
|
||
|
|
||
| def test_import_is_side_effect_free(): | ||
| """`import appstate` must construct nothing and touch no disk. | ||
|
|
||
| This is why the ~49 fixtures that `sys.modules.pop("server")` and re-import | ||
| (to rebuild `meta_db` under a patched CONFIG_DIR) keep working untouched: | ||
| server owns construction, appstate only mirrors it. A singleton *owned* | ||
| here would survive that pop and go stale. | ||
| """ | ||
| sys.modules.pop("appstate", None) | ||
| fresh = importlib.import_module("appstate") | ||
| try: | ||
| assert fresh.meta_db is None | ||
| assert fresh.audio_effect_mappings is None | ||
| finally: | ||
| sys.modules["appstate"] = appstate | ||
|
|
||
|
|
||
| def test_configure_publishes_known_slots(): | ||
| sentinel = object() | ||
| original = appstate.meta_db | ||
| try: | ||
| appstate.configure(meta_db=sentinel) | ||
| assert appstate.meta_db is sentinel | ||
| finally: | ||
| appstate.configure(meta_db=original) | ||
|
|
||
|
|
||
| def test_configure_is_idempotent(): | ||
| """server re-imports call configure() again; the last write must win.""" | ||
| original = appstate.meta_db | ||
| try: | ||
| appstate.configure(meta_db="first") | ||
| appstate.configure(meta_db="second") | ||
| assert appstate.meta_db == "second" | ||
| finally: | ||
| appstate.configure(meta_db=original) | ||
|
|
||
|
|
||
| def test_configure_rejects_an_unknown_slot(): | ||
| """A typo'd or stale keyword must raise, not silently create a global that | ||
| nothing reads. A seam whose wiring can no-op undetected is worse than none.""" | ||
| with pytest.raises(TypeError, match="unknown slot"): | ||
| appstate.configure(met_db="typo") | ||
| assert not hasattr(appstate, "met_db") | ||
|
|
||
|
|
||
| def test_late_bound_read_sees_a_later_configure(): | ||
| """Routers must read `appstate.meta_db`, never `from appstate import meta_db`. | ||
| This pins the property that makes that rule work.""" | ||
| def router_style_read(): | ||
| return appstate.meta_db # module attribute, resolved at call time | ||
|
|
||
| original = appstate.meta_db | ||
| try: | ||
| appstate.configure(meta_db="before") | ||
| assert router_style_read() == "before" | ||
| appstate.configure(meta_db="after") | ||
| assert router_style_read() == "after" | ||
| finally: | ||
| appstate.configure(meta_db=original) | ||
|
|
||
|
|
||
| def test_server_wires_the_seam(isolated_server): | ||
| """The one that catches a dropped `appstate.configure(...)` call. | ||
|
|
||
| Identity, not truthiness, so a stray re-assignment or a half-applied edit | ||
| fails here rather than in some router months later. | ||
| """ | ||
| assert appstate.meta_db is isolated_server.meta_db | ||
| assert appstate.audio_effect_mappings is isolated_server.audio_effect_mappings | ||
| assert appstate.meta_db is not None | ||
|
|
||
|
|
||
| def test_reimporting_server_republishes_the_fresh_singletons( | ||
| isolated_server, tmp_path, monkeypatch | ||
| ): | ||
| """The 49-fixture contract, exercised end to end. | ||
|
|
||
| Those fixtures `sys.modules.pop("server")` + re-import to rebuild `meta_db` | ||
| under a new CONFIG_DIR, and know nothing about appstate. So the seam must | ||
| re-publish on that second import. This is the test that would fail if | ||
| `appstate` ever *owned* the singletons: a module-level `meta_db` there | ||
| survives the pop and the assertions below would still see the FIRST DB. | ||
| """ | ||
| first_db = isolated_server.meta_db | ||
| assert appstate.meta_db is first_db | ||
| assert str(tmp_path) in first_db.db_path | ||
|
|
||
| second_config = tmp_path / "second" | ||
| monkeypatch.setenv("CONFIG_DIR", str(second_config)) | ||
| sys.modules.pop("server", None) | ||
| second_server = importlib.import_module("server") | ||
| try: | ||
| assert second_server.meta_db is not first_db # genuinely rebuilt | ||
| assert str(second_config) in second_server.meta_db.db_path | ||
| assert appstate.meta_db is second_server.meta_db # ...and re-published | ||
| assert appstate.audio_effect_mappings is second_server.audio_effect_mappings | ||
| finally: | ||
| _close_server_dbs(second_server) | ||
| sys.modules.pop("server", None) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.