test(fence): the unit fence has no exemptions, and the owner filter is held in every module
The per-file pin list and the unit-by-unit switch go: every route file is held to zero database work, every unit's package to its edges, and core code says whose a row is through userscope too. CLAUDE.md, the backend README and the gate register describe the units as they now are.
This commit is contained in:
@@ -5,17 +5,14 @@ What it may not do is build SQL, call the session's data methods, change a model
|
||||
import a unit's tables: when a route did those, ownership, the commit and the conditional `WHERE`
|
||||
lived in a different place in each handler, and a fix in one never reached its siblings.
|
||||
|
||||
The rule is switched on unit by unit. `ALLOWED` pins what each HTTP file still does today, and the
|
||||
pin is exact: a count that rises fails (new database work in a route), and one that falls fails
|
||||
until its entry is lowered (the fence only tightens, as `baseline.json` does for basedpyright). An
|
||||
empty `ALLOWED` is the end state — a file listed there is a debt, not an exemption.
|
||||
The rule has no exemptions: a single `select()` in a unit's route file fails it.
|
||||
|
||||
A core route (identity, admin, ops) is not a unit's HTTP layer and may use the session; it may
|
||||
still not reach into a unit's tables. A route may NAME a unit's model as a type (under
|
||||
`TYPE_CHECKING`), never use one as a value.
|
||||
|
||||
Below the HTTP layer, a fenced unit's own code keeps to its edges (`FENCED`, `READS`): see the
|
||||
second half of this file.
|
||||
Below the HTTP layer, a unit's own code keeps to its edges (`READS`): see the second half of this
|
||||
file. Whose a row is, every module says through `userscope` — a core one too.
|
||||
"""
|
||||
|
||||
import ast
|
||||
@@ -107,10 +104,6 @@ UNIT_ROUTES = {
|
||||
"plex/": "plex",
|
||||
}
|
||||
|
||||
# What each HTTP file still does, by file: the debt a unit's sprint pays off. Exact counts. A core
|
||||
# route's count is only the unit tables it imports; it is paid by the sprint of the unit it reaches.
|
||||
ALLOWED: dict[str, int] = {}
|
||||
|
||||
# The session methods that read or write data. `commit` and `rollback` stay with the handler.
|
||||
SESSION_DATA = {
|
||||
"add",
|
||||
@@ -257,19 +250,13 @@ def test_every_listed_route_exists():
|
||||
rels = [rel for rel, _ in _http_files()]
|
||||
for key in UNIT_ROUTES:
|
||||
assert any(rel == key or (key.endswith("/") and rel.startswith(key)) for rel in rels), key
|
||||
assert set(ALLOWED) <= set(rels), set(ALLOWED) - set(rels)
|
||||
|
||||
|
||||
def test_no_http_file_does_database_work_beyond_its_pin():
|
||||
drift = {}
|
||||
for rel, src in _http_files():
|
||||
found = violations(src, _unit_of(rel))
|
||||
pinned = ALLOWED.get(rel, 0)
|
||||
if len(found) != pinned:
|
||||
drift[rel] = (pinned, len(found), found if len(found) > pinned else "lower its pin")
|
||||
assert drift == {}, (
|
||||
"an HTTP file's database work moved off its pin (pinned, now, what): move new work into"
|
||||
" the unit's queries/service; when a count fell, lower ALLOWED to it"
|
||||
def test_no_http_file_does_database_work():
|
||||
hits = {rel: found for rel, src in _http_files() if (found := violations(src, _unit_of(rel)))}
|
||||
assert hits == {}, (
|
||||
"a route does database work: reads go through the unit's queries, writes through its"
|
||||
" service; a core route reaches a unit's tables through that unit's queries too"
|
||||
)
|
||||
|
||||
|
||||
@@ -307,11 +294,11 @@ def test_the_detector_sees_each_kind_of_database_work():
|
||||
|
||||
|
||||
# --- inside a unit: its package --------------------------------------------------------------
|
||||
# A unit whose sprint has landed is fenced beyond its HTTP layer too. Its own code reads another
|
||||
# unit's tables only along a declared edge (the feed joins Video with VideoState in one query; a
|
||||
# boundary that split it would cost the speed the app is for), and writes another unit's tables
|
||||
# never — that goes through the owner's service. Whose a row is, it says through `userscope`, and
|
||||
# it commits only where a job owns its session.
|
||||
# Every unit is fenced beyond its HTTP layer too. Its own code reads another unit's tables only
|
||||
# along a declared edge (the feed joins Video with VideoState in one query; a boundary that split
|
||||
# it would cost the speed the app is for), and writes another unit's tables never — that goes
|
||||
# through the owner's service. It commits only where a job owns its session. Whose a row is, it
|
||||
# says through `userscope`, and so does core code: the owner filter is checked in every module.
|
||||
|
||||
# Each unit's code outside the HTTP layer: its package, and the entry point that runs its jobs.
|
||||
UNIT_PACKAGES = {
|
||||
@@ -341,9 +328,7 @@ UNIT_PACKAGES = {
|
||||
# pages' reads and writes.
|
||||
"plex": ("plex/",),
|
||||
}
|
||||
# The units fenced so far; the rest join with their sprint.
|
||||
FENCED = {"capture", "reader", "catalog", "plex"}
|
||||
# Whose tables a unit's own code may READ. Core code reads a fenced unit's tables through its queries.
|
||||
# Whose tables a unit's own code may READ. Core code reads a unit's tables through its queries.
|
||||
READS = {
|
||||
"capture": {"catalog", "plex"},
|
||||
"reader": {"catalog", "plex"},
|
||||
@@ -352,7 +337,7 @@ READS = {
|
||||
}
|
||||
# The columns that say whose a row is.
|
||||
OWNER_COLUMNS = {"user_id", "to_user_id", "from_user_id", "created_by"}
|
||||
# Where a fenced unit's code may commit: the jobs that own their session (the worker and what it
|
||||
# Where a unit's code may commit: the jobs that own their session (the worker and what it
|
||||
# runs, the GC, the backfill — none of them inside a request), and the one write that must commit
|
||||
# where it stands: an upload's rows and the file it placed on disk land together or not at all, so
|
||||
# a failed commit can remove the file (`upload.ingest_upload`). Everything else flushes; the handler
|
||||
@@ -465,14 +450,14 @@ def _enclosing_functions(tree: ast.AST) -> dict[ast.AST, str]:
|
||||
|
||||
|
||||
def package_violations(rel: str, source: str) -> list[str]:
|
||||
"""What a module outside the HTTP layer does against a fenced unit's boundary, as `line: what`."""
|
||||
"""What a module outside the HTTP layer does against a unit's boundary, as `line: what`."""
|
||||
tree = ast.parse(source)
|
||||
mine = _package_unit(rel)
|
||||
found = []
|
||||
models = _imported_models(tree)
|
||||
for name in models.values():
|
||||
theirs = UNIT_MODEL_NAMES[name]
|
||||
if theirs == mine or (mine not in FENCED and theirs not in FENCED):
|
||||
if theirs == mine:
|
||||
continue
|
||||
if theirs not in READS.get(mine or "", set()):
|
||||
found.append(f"0: {mine or 'core'} imports {theirs}'s {name}")
|
||||
@@ -487,10 +472,10 @@ def package_violations(rel: str, source: str) -> list[str]:
|
||||
if written is None:
|
||||
continue
|
||||
theirs = UNIT_MODEL_NAMES[written]
|
||||
if theirs != mine and (mine in FENCED or theirs in FENCED):
|
||||
if theirs != mine:
|
||||
found.append(f"{node.lineno}: {mine or 'core'} writes {theirs}'s {written}")
|
||||
if mine in FENCED:
|
||||
found += _owner_filters(tree)
|
||||
found += _owner_filters(tree)
|
||||
if mine is not None:
|
||||
functions = _enclosing_functions(tree)
|
||||
allowed = UNIT_COMMITS.get(mine, set())
|
||||
for node in ast.walk(tree):
|
||||
@@ -522,7 +507,7 @@ def _owner_filters(tree: ast.AST) -> list[str]:
|
||||
return found
|
||||
|
||||
|
||||
def test_a_fenced_units_code_keeps_to_its_edges():
|
||||
def test_a_units_code_keeps_to_its_edges():
|
||||
hits = {rel: found for rel, src in _app_files() if (found := package_violations(rel, src))}
|
||||
assert hits == {}, (
|
||||
"read another unit's tables only along READS, write them only through their service, say"
|
||||
@@ -530,12 +515,8 @@ def test_a_fenced_units_code_keeps_to_its_edges():
|
||||
)
|
||||
|
||||
|
||||
def test_a_fenced_units_routes_filter_owners_through_userscope():
|
||||
hits = {
|
||||
rel: found
|
||||
for rel, src in _http_files()
|
||||
if _unit_of(rel) in FENCED and (found := _owner_filters(ast.parse(src)))
|
||||
}
|
||||
def test_every_route_filters_owners_through_userscope():
|
||||
hits = {rel: found for rel, src in _http_files() if (found := _owner_filters(ast.parse(src)))}
|
||||
assert hits == {}
|
||||
|
||||
|
||||
@@ -562,6 +543,7 @@ PACKAGE_CASES = {
|
||||
("sync/videos.py", "from app.models import Playlist"): 1, # no catalog→reader edge
|
||||
("downloads/x.py", "q = DownloadJob.user_id == 1"): 1,
|
||||
("downloads/x.py", "q = owned_by(DownloadJob.user_id, 1)"): 0,
|
||||
("quota.py", "q = QuotaEvent.user_id == 1"): 1, # core too
|
||||
("downloads/x.py", "def f(db):\n db.commit()"): 1,
|
||||
("downloads/gc.py", "def f(db):\n db.commit()"): 0,
|
||||
("downloads/upload.py", "def ingest_upload(db):\n db.commit()"): 0,
|
||||
|
||||
Reference in New Issue
Block a user