mirror of
https://github.com/CyberMind-FR/secubox-deb.git
synced 2026-07-28 21:17:36 +00:00
fix(ci): cache-lint recognizes threading.Thread background-refresh idiom
secubox-frigate follows the double-cache pattern (daemon refresh thread + in-memory _cache guard + file cache + compute fallback) but used a sync handler + threading.Thread(target=refresh_cache) instead of asyncio.create_task — which the lint's has_bg_refresh detection didn't recognize → false-positive [no-cache-signals] → Dashboard Cache Lint red. Detect threading.Thread(target= refresh_*)/Thread(target=refresh_*) alongside asyncio.create_task. +self-test. Co-Authored-By: Gerald KERMA <devel@cybermind.fr>
This commit is contained in:
parent
bd8e2e382e
commit
68a3e78673
|
|
@ -188,10 +188,20 @@ def _module_signals(tree: ast.Module) -> tuple[bool, bool]:
|
|||
|
||||
- has_cache_instance: a module-level assignment whose target is a
|
||||
Name ending in ``_cache`` (e.g. ``stats_cache = StatsCache(...)``).
|
||||
- has_bg_refresh: any ``asyncio.create_task(refresh_*())`` or
|
||||
``@app.on_event("startup")`` that spawns a coroutine whose name
|
||||
contains ``refresh`` / ``cache`` / ``poll`` / ``loop``.
|
||||
- has_bg_refresh: a background refresh spawner, either
|
||||
* ``asyncio.create_task(refresh_*())`` (async idiom), or
|
||||
* ``threading.Thread(target=refresh_*)`` / ``Thread(target=refresh_*)``
|
||||
(the sync idiom — a daemon thread refreshing a module-level cache,
|
||||
used by sync FastAPI handlers, e.g. secubox-frigate),
|
||||
where the spawned callable's name contains ``refresh`` / ``cache`` /
|
||||
``poll`` / ``loop`` / ``periodic``.
|
||||
"""
|
||||
_BG_KW = ("refresh", "cache", "poll", "loop", "periodic")
|
||||
|
||||
def _name_of(n) -> str:
|
||||
return (n.id if isinstance(n, ast.Name)
|
||||
else n.attr if isinstance(n, ast.Attribute) else "")
|
||||
|
||||
has_cache = False
|
||||
has_bg = False
|
||||
|
||||
|
|
@ -204,23 +214,25 @@ def _module_signals(tree: ast.Module) -> tuple[bool, bool]:
|
|||
# Background refresh detection
|
||||
if isinstance(node, ast.Call):
|
||||
func = node.func
|
||||
# (a) asyncio.create_task(refresh_*())
|
||||
if (
|
||||
isinstance(func, ast.Attribute)
|
||||
and func.attr == "create_task"
|
||||
and isinstance(func.value, ast.Name)
|
||||
and func.value.id == "asyncio"
|
||||
):
|
||||
# Inspect the argument: is it a refresh-style coroutine?
|
||||
if node.args and isinstance(node.args[0], ast.Call):
|
||||
inner = node.args[0].func
|
||||
inner_name = (
|
||||
inner.id if isinstance(inner, ast.Name)
|
||||
else inner.attr if isinstance(inner, ast.Attribute)
|
||||
else ""
|
||||
)
|
||||
if any(kw in inner_name.lower()
|
||||
for kw in ("refresh", "cache", "poll", "loop", "periodic")):
|
||||
inner_name = _name_of(node.args[0].func)
|
||||
if any(kw in inner_name.lower() for kw in _BG_KW):
|
||||
has_bg = True
|
||||
# (b) threading.Thread(target=refresh_*) or Thread(target=refresh_*)
|
||||
if (isinstance(func, ast.Attribute) and func.attr == "Thread") or \
|
||||
(isinstance(func, ast.Name) and func.id == "Thread"):
|
||||
for kw in node.keywords:
|
||||
if kw.arg == "target":
|
||||
tgt_name = _name_of(kw.value)
|
||||
if any(k in tgt_name.lower() for k in _BG_KW):
|
||||
has_bg = True
|
||||
|
||||
return has_cache, has_bg
|
||||
|
||||
|
|
|
|||
|
|
@ -140,6 +140,33 @@ def test_handler_with_bg_refresh_task_is_compliant(tmp_path):
|
|||
assert not unjustified
|
||||
|
||||
|
||||
def test_handler_with_thread_refresh_is_compliant(tmp_path):
|
||||
"""``threading.Thread(target=refresh_cache).start()`` — the SYNC background
|
||||
refresh idiom (sync FastAPI handler + daemon thread, e.g. secubox-frigate) —
|
||||
covers all hot routes just like the async create_task pattern."""
|
||||
pkg = make_fake_packages(tmp_path, {
|
||||
"secubox-thread": """
|
||||
import threading
|
||||
from fastapi import FastAPI
|
||||
app = FastAPI()
|
||||
|
||||
def refresh_cache():
|
||||
pass
|
||||
|
||||
@app.on_event("startup")
|
||||
def startup():
|
||||
threading.Thread(target=refresh_cache, daemon=True).start()
|
||||
|
||||
@app.get("/stats")
|
||||
def get_stats():
|
||||
from pathlib import Path
|
||||
return Path("/var/cache/foo.json").read_text()
|
||||
""",
|
||||
})
|
||||
_, unjustified = cdc.run(pkg, Path("/nonexistent"))
|
||||
assert not unjustified
|
||||
|
||||
|
||||
def test_non_hot_route_is_not_audited(tmp_path):
|
||||
"""``/health`` is NOT in HOT_ROUTES — even with I/O it should not flag."""
|
||||
pkg = make_fake_packages(tmp_path, {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user