peter
3a688fa0f5
The nine Capture tables (files, jobs, folders, shares, links, quota, cookies, profiles, library resume points) leave app/models.py for the unit's own module; app.tables registers every unit's tables for the migrations, the test database and the ownership check. No schema change.
308 lines
11 KiB
Python
308 lines
11 KiB
Python
"""The HTTP layer of a unit does no database work of its own, held in code rather than prose.
|
|
|
|
A route validates, calls its unit's `queries` (reads) or `service` (writes), serializes and commits.
|
|
What it may not do is build SQL, call the session's data methods, change a model's attributes or
|
|
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.
|
|
|
|
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.
|
|
"""
|
|
|
|
import ast
|
|
from pathlib import Path
|
|
|
|
import app.tables # noqa: F401 — every unit's tables, for the ownership check
|
|
from app.db import Base
|
|
|
|
APP = Path(__file__).parent.parent / "app"
|
|
ROUTES = APP / "routes"
|
|
|
|
# Each unit's tables. A new model is placed here (or in CORE_MODELS) by the test below, so a table
|
|
# never joins the schema without an owner.
|
|
UNIT_MODELS = {
|
|
"capture": {
|
|
"MediaAsset",
|
|
"DownloadProfile",
|
|
"FileFolder",
|
|
"DownloadJob",
|
|
"DownloadShare",
|
|
"DownloadQuota",
|
|
"DownloadLink",
|
|
"DownloadCredential",
|
|
"LibraryState",
|
|
},
|
|
"reader": {
|
|
"VideoState",
|
|
"Playlist",
|
|
"PlaylistItem",
|
|
"SavedView",
|
|
"ComfortPin",
|
|
"Tag",
|
|
"ChannelTag",
|
|
},
|
|
"catalog": {
|
|
"Channel",
|
|
"Subscription",
|
|
"Video",
|
|
"SearchFind",
|
|
"ExploredChannel",
|
|
"BlockedChannel",
|
|
},
|
|
"plex": {
|
|
"PlexLibrary",
|
|
"PlexShow",
|
|
"PlexSeason",
|
|
"PlexItem",
|
|
"PlexPerson",
|
|
"PlexCollection",
|
|
"PlexState",
|
|
"PlexLink",
|
|
"PlexPlaylist",
|
|
"PlexPlaylistItem",
|
|
},
|
|
}
|
|
CORE_MODELS = {
|
|
"User",
|
|
"OAuthToken",
|
|
"AuthToken",
|
|
"Invite",
|
|
"ApiQuotaUsage",
|
|
"QuotaEvent",
|
|
"AuditLog",
|
|
"LogEvent",
|
|
"DebugSession",
|
|
"DebugScreenshot",
|
|
"AppState",
|
|
"JobRun",
|
|
"SystemConfig",
|
|
"Notification",
|
|
"ApiToken",
|
|
"ApiTokenGrant",
|
|
}
|
|
|
|
# Which unit each HTTP file belongs to (path relative to app/routes; a folder covers its files).
|
|
# Anything not listed is a core route.
|
|
UNIT_ROUTES = {
|
|
"downloads.py": "capture",
|
|
"files.py": "capture",
|
|
"public.py": "capture",
|
|
"feed.py": "reader",
|
|
"playlists.py": "reader",
|
|
"saved_views.py": "reader",
|
|
"discover.py": "reader",
|
|
"tags.py": "reader",
|
|
"channels.py": "catalog",
|
|
"search.py": "catalog",
|
|
"sync.py": "catalog",
|
|
"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 = {
|
|
"channels.py": 83,
|
|
"discover.py": 7,
|
|
"downloads.py": 88,
|
|
"feed.py": 36,
|
|
"files.py": 58,
|
|
"playlists.py": 72,
|
|
"plex/_shared.py": 1,
|
|
"plex/admin.py": 12,
|
|
"plex/browse.py": 93,
|
|
"plex/collections.py": 33,
|
|
"plex/detail.py": 29,
|
|
"plex/discover.py": 44,
|
|
"plex/people.py": 20,
|
|
"plex/playback.py": 42,
|
|
"plex/playlists.py": 80,
|
|
"public.py": 12,
|
|
"runs.py": 1,
|
|
"saved_views.py": 17,
|
|
"scheduler.py": 3,
|
|
"search.py": 46,
|
|
"sync.py": 38,
|
|
"tags.py": 10,
|
|
"tokens.py": 1,
|
|
}
|
|
|
|
# The session methods that read or write data. `commit` and `rollback` stay with the handler.
|
|
SESSION_DATA = {
|
|
"add",
|
|
"add_all",
|
|
"delete",
|
|
"execute",
|
|
"get",
|
|
"merge",
|
|
"flush",
|
|
"refresh",
|
|
"query",
|
|
"scalar",
|
|
"scalars",
|
|
"expunge",
|
|
}
|
|
# What a route may name from SQLAlchemy: the session's type, for its `db: Session` parameter, and
|
|
# the errors its commit raises (the commit is the handler's, so is catching what it throws).
|
|
SQLALCHEMY_ALLOWED = {"Session"}
|
|
SQLALCHEMY_ALLOWED_MODULES = {"sqlalchemy.exc"}
|
|
# Objects a route owns and may set attributes on; anything else it writes to is a model.
|
|
OWN_OBJECTS = {"self", "cls", "request", "response"}
|
|
UNIT_MODEL_NAMES = {name: unit for unit, names in UNIT_MODELS.items() for name in names}
|
|
|
|
|
|
def _unit_of(rel: str) -> str | None:
|
|
for key, unit in UNIT_ROUTES.items():
|
|
if rel == key or (key.endswith("/") and rel.startswith(key)):
|
|
return unit
|
|
return None
|
|
|
|
|
|
def _session_names(tree: ast.AST) -> set[str]:
|
|
"""The names a module binds to a database session: a parameter typed `Session`, or a name
|
|
assigned (or `with`-bound) from `SessionLocal()`."""
|
|
names = set()
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.arg) and isinstance(node.annotation, ast.Name):
|
|
if node.annotation.id == "Session":
|
|
names.add(node.arg)
|
|
made = None
|
|
if isinstance(node, ast.Assign) and len(node.targets) == 1:
|
|
made, target = node.value, node.targets[0]
|
|
elif isinstance(node, ast.withitem):
|
|
made, target = node.context_expr, node.optional_vars
|
|
if (
|
|
isinstance(made, ast.Call)
|
|
and isinstance(made.func, ast.Name)
|
|
and made.func.id == "SessionLocal"
|
|
and isinstance(target, ast.Name)
|
|
):
|
|
names.add(target.id)
|
|
return names
|
|
|
|
|
|
def _root(node: ast.expr) -> ast.expr:
|
|
while isinstance(node, (ast.Attribute, ast.Subscript)):
|
|
node = node.value
|
|
return node
|
|
|
|
|
|
def violations(source: str, unit: str | None) -> list[str]:
|
|
"""Each piece of database work in an HTTP file, as `line: what`. A core route (`unit` None) is
|
|
held only to the last rule: no unit's tables."""
|
|
tree = ast.parse(source)
|
|
found = []
|
|
sql_names = set()
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.ImportFrom):
|
|
module = node.module or ""
|
|
for alias in node.names:
|
|
bound = alias.asname or alias.name
|
|
if (
|
|
module.split(".")[0] == "sqlalchemy"
|
|
and module not in SQLALCHEMY_ALLOWED_MODULES
|
|
and alias.name not in SQLALCHEMY_ALLOWED
|
|
):
|
|
sql_names.add(bound)
|
|
if (
|
|
alias.name in UNIT_MODEL_NAMES
|
|
and module.startswith("app.")
|
|
and "models" in module
|
|
):
|
|
found.append(
|
|
f"{node.lineno}: imports {UNIT_MODEL_NAMES[alias.name]}'s {alias.name}"
|
|
)
|
|
elif isinstance(node, ast.Import):
|
|
for alias in node.names:
|
|
if alias.name.split(".")[0] == "sqlalchemy":
|
|
sql_names.add((alias.asname or alias.name).split(".")[0])
|
|
if unit is None:
|
|
return found
|
|
sessions = _session_names(tree)
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load) and node.id in sql_names:
|
|
found.append(f"{node.lineno}: uses sqlalchemy's {node.id}")
|
|
if isinstance(node, ast.Call):
|
|
f = node.func
|
|
if isinstance(f, ast.Attribute) and isinstance(f.value, ast.Name):
|
|
if f.value.id in sessions and f.attr in SESSION_DATA:
|
|
found.append(f"{node.lineno}: {f.value.id}.{f.attr}()")
|
|
targets = []
|
|
if isinstance(node, ast.Assign):
|
|
targets = node.targets
|
|
elif isinstance(node, (ast.AugAssign, ast.AnnAssign)):
|
|
targets = [node.target]
|
|
for target in targets:
|
|
if isinstance(target, ast.Attribute):
|
|
base = _root(target)
|
|
if not (isinstance(base, ast.Name) and base.id in OWN_OBJECTS):
|
|
found.append(f"{node.lineno}: sets .{target.attr}")
|
|
return sorted(found, key=lambda v: int(v.split(":")[0]))
|
|
|
|
|
|
def _http_files():
|
|
for path in sorted(ROUTES.rglob("*.py")):
|
|
rel = path.relative_to(ROUTES).as_posix()
|
|
yield rel, path.read_text(encoding="utf-8")
|
|
|
|
|
|
def test_every_table_has_an_owner():
|
|
tables = {mapper.class_.__name__ for mapper in Base.registry.mappers}
|
|
owned = set(UNIT_MODEL_NAMES) | CORE_MODELS
|
|
assert tables - owned == set(), "a model with no unit: place it in UNIT_MODELS or CORE_MODELS"
|
|
assert owned - tables == set(), "UNIT_MODELS/CORE_MODELS name a model that no longer exists"
|
|
assert not set(UNIT_MODEL_NAMES) & CORE_MODELS
|
|
|
|
|
|
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"
|
|
)
|
|
|
|
|
|
# The detector itself: each kind of database work it must see, and what it must leave alone.
|
|
DETECTOR_CASES = {
|
|
"from sqlalchemy import select\nq = select(1)": 1,
|
|
"import sqlalchemy as sa\nsa.select(1).where(sa.true())": 2,
|
|
"from sqlalchemy import func\nn = func.count()": 1,
|
|
"from sqlalchemy.orm import Session\ndef f(db: Session): db.commit(); db.rollback()": 0,
|
|
"from sqlalchemy.orm import Session\ndef f(db: Session): db.add(x)": 1,
|
|
"def f(conn: Session): conn.execute(q)": 1,
|
|
"from app.db import SessionLocal\nwith SessionLocal() as s:\n s.get(X, 1)": 1,
|
|
"from app.db import SessionLocal\ns = SessionLocal()\ns.scalars(q)": 1,
|
|
"def f(job): job.status = 'x'": 1,
|
|
"def f(job): job.meta['k'] = 1": 0,
|
|
"def f(job): job.parts.count += 1": 1,
|
|
"def f(self, request, response): self.x = 1; request.state.y = 2; response.status_code = 3": 0,
|
|
"from app.models import DownloadJob, User": 1,
|
|
"from app.capture.models import DownloadJob as J": 1,
|
|
"def f(cache): cache.get('k')": 0,
|
|
"from sqlalchemy.exc import IntegrityError\ntry:\n pass\nexcept IntegrityError:\n pass": 0,
|
|
}
|
|
|
|
|
|
def test_the_detector_sees_each_kind_of_database_work():
|
|
for source, expected in DETECTOR_CASES.items():
|
|
assert len(violations(source, "capture")) == expected, source
|
|
assert violations("from app.models import Video\nfrom sqlalchemy import select", None) == [
|
|
"1: imports catalog's Video"
|
|
]
|