fix(cron): guard profile-switch polling races
This commit is contained in:
parent
1a64fc68dd
commit
67e527fd74
4 changed files with 75 additions and 76 deletions
|
|
@ -19902,19 +19902,8 @@ def _handle_cron_recent(handler, parsed):
|
|||
since = 0.0
|
||||
try:
|
||||
from cron.jobs import list_jobs
|
||||
from api.profiles import (
|
||||
cron_profile_context_for_home,
|
||||
get_active_profile_name,
|
||||
get_hermes_home_for_profile,
|
||||
)
|
||||
|
||||
active_profile = get_active_profile_name() or "default"
|
||||
active_home = get_hermes_home_for_profile(active_profile)
|
||||
# Match the default Tasks list scope explicitly. The recent-completion
|
||||
# endpoint feeds an actionable unread badge, so returning jobs from a
|
||||
# different profile can strand an ID that has no visible row to open.
|
||||
with cron_profile_context_for_home(active_home):
|
||||
jobs = list_jobs(include_disabled=True)
|
||||
jobs = list_jobs(include_disabled=True)
|
||||
completions = []
|
||||
for job in jobs:
|
||||
job_id = str(job.get("id", "") or "")
|
||||
|
|
|
|||
|
|
@ -12618,9 +12618,11 @@ async function disableAuth(){
|
|||
let _cronPollSince=Date.now()/1000; // track from page load
|
||||
let _cronPollTimer=null;
|
||||
let _cronUnreadCount=0;
|
||||
let _cronPollGeneration=0;
|
||||
const _cronNewJobIds=new Set(); // track which job IDs had new completions (unread)
|
||||
|
||||
function _resetCronUnreadForProfileSwitch(){
|
||||
_cronPollGeneration++;
|
||||
_cronNewJobIds.clear();
|
||||
_cronPollSince=Date.now()/1000;
|
||||
updateCronBadge();
|
||||
|
|
@ -12637,7 +12639,9 @@ function startCronPolling(){
|
|||
_cronPollTimer=setInterval(async()=>{
|
||||
if(document.hidden) return; // don't poll when tab is in background
|
||||
try{
|
||||
const pollGeneration=_cronPollGeneration;
|
||||
const data=await api(`/api/crons/recent?since=${_cronPollSince}`);
|
||||
if(pollGeneration!==_cronPollGeneration) return;
|
||||
if(data.completions&&data.completions.length>0){
|
||||
for(const c of data.completions){
|
||||
if(c.toast_notifications !== false){
|
||||
|
|
|
|||
|
|
@ -486,6 +486,7 @@ function extractFunc(name) {{
|
|||
let _cronPollSince = 10;
|
||||
let _cronPollTimer = null;
|
||||
let _cronUnreadCount = 0;
|
||||
let _cronPollGeneration = 0;
|
||||
const _cronNewJobIds = new Set();
|
||||
const markCalls = [];
|
||||
const toastCalls = [];
|
||||
|
|
|
|||
|
|
@ -2,81 +2,45 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import json
|
||||
import sys
|
||||
import types
|
||||
from contextlib import contextmanager
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
PANELS_JS = (ROOT / "static" / "panels.js").read_text(encoding="utf-8")
|
||||
ROUTES_PY = (ROOT / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
NODE = shutil.which("node")
|
||||
|
||||
|
||||
class _JSONHandler:
|
||||
def __init__(self):
|
||||
self.status = None
|
||||
self.response_headers = []
|
||||
self.wfile = io.BytesIO()
|
||||
|
||||
def send_response(self, status):
|
||||
self.status = status
|
||||
|
||||
def send_header(self, key, value):
|
||||
self.response_headers.append((key, value))
|
||||
|
||||
def end_headers(self):
|
||||
pass
|
||||
def _extract_function(source: str, name: str) -> str:
|
||||
start = source.index(f"function {name}(")
|
||||
brace = source.index("{", start)
|
||||
depth = 1
|
||||
pos = brace + 1
|
||||
while depth and pos < len(source):
|
||||
if source[pos] == "{":
|
||||
depth += 1
|
||||
elif source[pos] == "}":
|
||||
depth -= 1
|
||||
pos += 1
|
||||
assert depth == 0
|
||||
return source[start:pos]
|
||||
|
||||
|
||||
def _payload(handler):
|
||||
return json.loads(handler.wfile.getvalue().decode("utf-8"))
|
||||
def test_recent_handler_reuses_dispatcher_cron_context_without_nesting():
|
||||
dispatch_start = ROUTES_PY.index('if parsed.path == "/api/crons/recent":')
|
||||
dispatch_end = ROUTES_PY.index('if parsed.path == "/api/crons/status":', dispatch_start)
|
||||
dispatch = ROUTES_PY[dispatch_start:dispatch_end]
|
||||
handler_start = ROUTES_PY.index("def _handle_cron_recent(")
|
||||
handler_end = ROUTES_PY.index("\ndef ", handler_start + 1)
|
||||
handler = ROUTES_PY[handler_start:handler_end]
|
||||
|
||||
|
||||
def test_recent_completions_are_read_from_the_active_profile_home(monkeypatch):
|
||||
import api.profiles as profiles
|
||||
import api.routes as routes
|
||||
|
||||
entered_homes = []
|
||||
|
||||
@contextmanager
|
||||
def profile_context(home):
|
||||
entered_homes.append(home)
|
||||
yield
|
||||
|
||||
cron_pkg = types.ModuleType("cron")
|
||||
cron_pkg.__path__ = []
|
||||
cron_jobs = types.ModuleType("cron.jobs")
|
||||
|
||||
def list_jobs(include_disabled=True):
|
||||
assert entered_homes == ["/profiles/vops"]
|
||||
return [
|
||||
{
|
||||
"id": "vops-job",
|
||||
"name": "Vops job",
|
||||
"last_run_at": 20,
|
||||
"last_status": "success",
|
||||
}
|
||||
]
|
||||
|
||||
cron_jobs.list_jobs = list_jobs
|
||||
monkeypatch.setitem(sys.modules, "cron", cron_pkg)
|
||||
monkeypatch.setitem(sys.modules, "cron.jobs", cron_jobs)
|
||||
monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "vops")
|
||||
monkeypatch.setattr(
|
||||
profiles, "get_hermes_home_for_profile", lambda name: f"/profiles/{name}"
|
||||
)
|
||||
monkeypatch.setattr(profiles, "cron_profile_context_for_home", profile_context)
|
||||
monkeypatch.setattr(routes, "_latest_cron_session_info_for_jobs", lambda *args: {})
|
||||
|
||||
handler = _JSONHandler()
|
||||
routes._handle_cron_recent(handler, SimpleNamespace(query="since=10"))
|
||||
|
||||
assert handler.status == 200
|
||||
assert entered_homes == ["/profiles/vops"]
|
||||
assert [row["job_id"] for row in _payload(handler)["completions"]] == ["vops-job"]
|
||||
assert "with cron_profile_context():" in dispatch
|
||||
assert "cron_profile_context_for_home" not in handler
|
||||
|
||||
|
||||
def test_successful_profile_switch_resets_unread_cron_state():
|
||||
|
|
@ -91,6 +55,47 @@ def test_successful_profile_switch_resets_unread_cron_state():
|
|||
reset_start = PANELS_JS.index("function _resetCronUnreadForProfileSwitch(){")
|
||||
reset_end = PANELS_JS.index("\n}", reset_start)
|
||||
reset_body = PANELS_JS[reset_start:reset_end]
|
||||
assert "_cronPollGeneration++;" in reset_body
|
||||
assert "_cronNewJobIds.clear();" in reset_body
|
||||
assert "_cronPollSince=Date.now()/1000;" in reset_body
|
||||
assert "updateCronBadge();" in reset_body
|
||||
|
||||
|
||||
@pytest.mark.skipif(NODE is None, reason="node not on PATH")
|
||||
def test_poll_started_before_switch_cannot_recreate_unread_state():
|
||||
polling = _extract_function(PANELS_JS, "startCronPolling")
|
||||
reset = _extract_function(PANELS_JS, "_resetCronUnreadForProfileSwitch")
|
||||
script = f"""
|
||||
let _cronPollSince=10;
|
||||
let _cronPollTimer=null;
|
||||
let _cronUnreadCount=0;
|
||||
let _cronPollGeneration=0;
|
||||
const _cronNewJobIds=new Set();
|
||||
let intervalCallback=null;
|
||||
let resolveApi;
|
||||
global.document={{hidden:false}};
|
||||
global.setInterval=(callback)=>{{ intervalCallback=callback; return 1; }};
|
||||
global.api=()=>new Promise(resolve=>{{ resolveApi=resolve; }});
|
||||
global.showToast=()=>{{}};
|
||||
global.t=(key)=>key;
|
||||
global.updateCronBadge=()=>{{ _cronUnreadCount=_cronNewJobIds.size; }};
|
||||
{polling}
|
||||
{reset}
|
||||
startCronPolling();
|
||||
(async()=>{{
|
||||
const stalePoll=intervalCallback();
|
||||
_resetCronUnreadForProfileSwitch();
|
||||
resolveApi({{completions:[{{job_id:'old-profile-job',completed_at:20}}]}});
|
||||
await stalePoll;
|
||||
process.stdout.write(JSON.stringify({{
|
||||
unread:Array.from(_cronNewJobIds),
|
||||
count:_cronUnreadCount,
|
||||
generation:_cronPollGeneration,
|
||||
}}));
|
||||
}})().catch(error=>{{ console.error(error); process.exit(1); }});
|
||||
"""
|
||||
result = subprocess.run(
|
||||
[NODE, "-e", script], check=True, capture_output=True, text=True, timeout=30
|
||||
)
|
||||
state = json.loads(result.stdout)
|
||||
assert state == {"unread": [], "count": 0, "generation": 1}
|
||||
|
|
|
|||
Loading…
Reference in a new issue