From 606d8e944be3250a59fd83094e3848984bbbd4ba Mon Sep 17 00:00:00 2001 From: StyxX65 <150797939+StyxX65@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:03:12 +0200 Subject: [PATCH] Make Stop stop: no replay, no restart, immediate feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pressing Stop could look like the scan restarting, and for scheduled jobs it really did restart the Google stage. - Scheduler: check the abort flag between stages and never clear the Google abort after a Stop (that started a fresh all-users Google scan). A stopped job is recorded as "cancelled" and skips email/retention; scheduler_done carries cancelled=true. - SSE: scan_cancelled and file_scan_done now end the current scan like scan_done, and sse.replay_messages() never replays a finished scan. Previously a stopped M365 scan (and every file scan) stayed "current", so each reconnect replayed it and re-showed Stop + progress. - Frontend: scan_cancelled clears the M365 running flag and only tears down the UI when no Google/file scan is still running; scheduler_done / scheduler_error clear all running flags. - run_scan: Stop during collection now ends the scan (checkpoint, finish, scan_cancelled) instead of announcing scan_start / "Scanning…". - Stop button: switches to a disabled "Stopping…" state on click; all show/hide goes through _setStopBtnDisplay(), which resets it when hidden or when a new scan starts. Adds tests/test_stop_scan.py (10 tests). Co-Authored-By: Claude Opus 5.5 --- gdpr_scanner.py | 13 +-- lang/da.json | 4 + lang/de.json | 4 + lang/en.json | 4 + scan_engine.py | 39 +++---- scan_scheduler.py | 39 ++++--- sse.py | 28 +++++- static/js/CLAUDE.md | 5 + static/js/results.js | 2 +- static/js/scan.js | 77 ++++++++++---- static/js/scheduler.js | 3 +- tests/test_stop_scan.py | 218 ++++++++++++++++++++++++++++++++++++++++ 12 files changed, 372 insertions(+), 64 deletions(-) create mode 100644 tests/test_stop_scan.py diff --git a/gdpr_scanner.py b/gdpr_scanner.py index 2edfcf2..e8f525e 100644 --- a/gdpr_scanner.py +++ b/gdpr_scanner.py @@ -1527,16 +1527,9 @@ def local_ip(): def scan_stream(): q = queue.Queue(maxsize=512) _sse_queues.append(q) - # Filter replay buffer: only include events from the current scan - # (avoids replaying stale events from a previous scan) - replay_scan_id = _sse_mod._current_scan_id - buf = [] - if replay_scan_id: - for msg in list(_sse_buffer): - if f'"scan_id": "{replay_scan_id}"' in msg: - buf.append(msg) - else: - buf = list(_sse_buffer) + # Replay only the running scan's events, never those of a finished or + # stopped scan (see sse.replay_messages) + buf = _sse_mod.replay_messages() def generate(): try: yield ": connected\n\n" diff --git a/lang/da.json b/lang/da.json index 8c962a6..0fcf6d6 100644 --- a/lang/da.json +++ b/lang/da.json @@ -260,6 +260,10 @@ "m365_admin_note": "Viser kun din konto. For at liste alle brugere skal en administrator give samtykke til User.Read.All i Azure Portal, eller tilføj konti manuelt nedenfor.", "m365_btn_scan": "Scan", "m365_btn_stop": "Stop", + "m365_btn_stopping": "Stopper…", + "m365_scan_stopping": "Stopper scanning — færdiggør det aktuelle element…", + "m365_scan_stopped": "Scanning stoppet.", + "m365_sched_stopped": "stoppet", "m365_pill_flagged": "markerede", "m365_pill_scanned": "scannet", "m365_filter_all_sources": "Alle kilder", diff --git a/lang/de.json b/lang/de.json index a5fabbf..6232f72 100644 --- a/lang/de.json +++ b/lang/de.json @@ -260,6 +260,10 @@ "m365_admin_note": "Es wird nur Ihr Konto angezeigt. Um alle Benutzer aufzulisten, muss ein Administrator User.Read.All im Azure Portal genehmigen oder Konten manuell unten hinzufügen.", "m365_btn_scan": "Scannen", "m365_btn_stop": "Stop", + "m365_btn_stopping": "Wird gestoppt…", + "m365_scan_stopping": "Scan wird gestoppt — aktuelles Element wird abgeschlossen…", + "m365_scan_stopped": "Scan gestoppt.", + "m365_sched_stopped": "gestoppt", "m365_pill_flagged": "markiert", "m365_pill_scanned": "gescannt", "m365_filter_all_sources": "Alle Quellen", diff --git a/lang/en.json b/lang/en.json index ecbc541..3dd1cb9 100644 --- a/lang/en.json +++ b/lang/en.json @@ -260,6 +260,10 @@ "m365_admin_note": "Only showing your account. To list all users, an admin must grant User.Read.All consent in Azure Portal, or add accounts manually below.", "m365_btn_scan": "Scan", "m365_btn_stop": "Stop", + "m365_btn_stopping": "Stopping…", + "m365_scan_stopping": "Stopping scan — finishing the current item…", + "m365_scan_stopped": "Scan stopped.", + "m365_sched_stopped": "stopped", "m365_pill_flagged": "flagged", "m365_pill_scanned": "scanned", "m365_filter_all_sources": "All sources", diff --git a/scan_engine.py b/scan_engine.py index 1bef525..e1b7100 100644 --- a/scan_engine.py +++ b/scan_engine.py @@ -553,12 +553,6 @@ def run_scan(options: dict): if rem < 3600: return f"{int(rem/60)}m" return f"{int(rem/3600)}h" - def _check_abort(): - if _state._scan_abort.is_set(): - broadcast("scan_cancelled", {"completed": completed}) - return True - return False - def _broadcast_card(item_meta: dict, cprs: list, pii_counts: dict | None = None): card = { "id": item_meta.get("id", ""), @@ -1089,6 +1083,26 @@ def run_scan(options: dict): if scanned_ids: work_items = [(k, m, f) for k, m, f in work_items if m.get("id", "") not in scanned_ids] + def _finish_aborted(done: int) -> None: + """Stop requested: checkpoint, finalise the DB scan, announce it.""" + # Save checkpoint so scan can be resumed later + _save_checkpoint(ck_key, scanned_ids, _state.flagged_items, _state.scan_meta) + # Finalise the DB scan record so items found before the stop stay + # visible — the early return otherwise skips finish_scan below, + # stranding them (invisible to get_session_items / get_open_items). + if _db and _db_scan_id: + try: + _db.finish_scan(_db_scan_id, done) + except Exception as _e: + logger.error("[db] finish_scan (aborted) failed: %s", _e) + broadcast("scan_cancelled", {"completed": done - resumed_count}) + + # Stop pressed during collection: the per-user loops just break out, so + # stop here — announcing scan_start/"Scanning…" made Stop look like a restart. + if _state._scan_abort.is_set(): + _finish_aborted(resumed_count) + return + total = len(work_items) broadcast("scan_start", { "total": total + resumed_count, @@ -1107,17 +1121,8 @@ def run_scan(options: dict): _items_since_save = 0 idx = -1 while _work_q: - if _check_abort(): - # Save checkpoint so scan can be resumed later - _save_checkpoint(ck_key, scanned_ids, _state.flagged_items, _state.scan_meta) - # Finalise the DB scan record so items found before the stop stay - # visible — this early return otherwise skips finish_scan below, - # stranding them (invisible to get_session_items / get_open_items). - if _db and _db_scan_id: - try: - _db.finish_scan(_db_scan_id, resumed_count + idx + 1) - except Exception as _e: - logger.error("[db] finish_scan (aborted) failed: %s", _e) + if _state._scan_abort.is_set(): + _finish_aborted(resumed_count + idx + 1) return idx += 1 kind, meta, _ = _work_q.popleft() # releases this item from the deque immediately diff --git a/scan_scheduler.py b/scan_scheduler.py index 0483bfe..6651c02 100644 --- a/scan_scheduler.py +++ b/scan_scheduler.py @@ -351,6 +351,9 @@ class ScanScheduler: ) _fs_extra = {k: _profile_opts[k] for k in _FS_OPT_KEYS if k in _profile_opts} for fs in options.get("file_sources", []): + # Stop pressed during an earlier stage — don't start more stages + if _m._scan_abort.is_set(): + break # Resolve string IDs to full source dicts if isinstance(fs, str): fs = _all_file_sources.get(fs, {"path": fs, "label": fs}) @@ -374,7 +377,12 @@ class ScanScheduler: # Legacy profiles store everything in sources[] google_sources = [s for s in options.get("sources", []) if s in ("gmail", "gdrive")] - if google_sources and state.google_connector: + # POST /api/scan/stop sets both abort events. _scan_abort was cleared + # when this job started, so if it is set now the user pressed Stop + # during an earlier stage — the Google stage must not start, and its + # abort event must not be cleared (that turned Stop into a restart). + stopped = _m._scan_abort.is_set() + if google_sources and state.google_connector and not stopped: from routes.google_scan import ( _run_google_scan as _rgs, _scan_lock as _gsl, @@ -383,12 +391,15 @@ class ScanScheduler: if _gsl.acquire(blocking=False): try: _gsa.clear() - logger.info("[scheduler] Starting Google scan — sources=%s", google_sources) - _rgs({ - "sources": google_sources, - "user_emails": [], # empty → scan all workspace users - "options": options.get("options", {}), - }) + # Stop sets _scan_abort first, so re-check after clearing: + # a Stop landing in between must still win. + if not _m._scan_abort.is_set(): + logger.info("[scheduler] Starting Google scan — sources=%s", google_sources) + _rgs({ + "sources": google_sources, + "user_emails": [], # empty → scan all workspace users + "options": options.get("options", {}), + }) except Exception as _ge: import traceback as _tb3 logger.error("[scheduler] Google scan error: %s\n%s", _ge, _tb3.format_exc()) @@ -399,18 +410,21 @@ class ScanScheduler: run["flagged"] = len(_m.flagged_items) run["scanned"] = _m.scan_meta.get("total_scanned", 0) - run["status"] = "completed" - logger.info("[scheduler] Job '%s' completed — %d flagged, %d scanned", - job_cfg.get("name", ""), run["flagged"], run["scanned"]) + stopped = _m._scan_abort.is_set() + run["status"] = "cancelled" if stopped else "completed" + logger.info("[scheduler] Job '%s' %s — %d flagged, %d scanned", + job_cfg.get("name", ""), run["status"], run["flagged"], run["scanned"]) - if job_cfg.get("auto_email") and state.flagged_items: + # A stopped job's results are partial — don't email them or apply + # retention based on them. + if not stopped and job_cfg.get("auto_email") and state.flagged_items: try: self._send_email_report(job_cfg) run["emailed"] = 1 except Exception as e: run["error"] = f"Scan OK, email failed: {e}" - if job_cfg.get("auto_retention") and job_cfg.get("retention_years"): + if not stopped and job_cfg.get("auto_retention") and job_cfg.get("retention_years"): try: self._run_retention(job_cfg) except Exception as e: @@ -420,6 +434,7 @@ class ScanScheduler: _m.broadcast("scheduler_done", { "flagged": run["flagged"], "scanned": run["scanned"], "emailed": run["emailed"], "job_name": job_cfg.get("name", ""), + "cancelled": stopped, }) except Exception as e: diff --git a/sse.py b/sse.py index 8ab08a7..52c5b44 100644 --- a/sse.py +++ b/sse.py @@ -19,6 +19,29 @@ logger = logging.getLogger(__name__) _sse_queues: list = [] _sse_buffer: deque = deque(maxlen=500) _current_scan_id: str = "" +# Scans that have ended (done / stopped). Their events stay in the buffer but +# are never replayed: replaying a stopped scan re-showed the Stop button and +# progress bar on every reconnect, so Stop looked like a restart. +_finished_scan_ids: deque = deque(maxlen=20) + +# Events that end the scan identified by _current_scan_id. +# scan_done: M365 finished; scan_cancelled: M365 stopped; file_scan_done: +# file scan finished or stopped. (Google scans don't set _current_scan_id.) +_TERMINAL_EVENTS = frozenset({"scan_done", "scan_cancelled", "file_scan_done"}) + +def replay_messages() -> list[str]: + """Buffered messages to replay to a newly connected browser. + + While a scan runs, only that scan's events. Otherwise everything except + events of scans that have already ended (done or stopped). + """ + buf = list(_sse_buffer) + if _current_scan_id: + tag = f'"scan_id": "{_current_scan_id}"' + return [m for m in buf if tag in m] + finished = [f'"scan_id": "{sid}"' for sid in _finished_scan_ids] + return [m for m in buf if not any(tag in m for tag in finished)] + def broadcast(event: str, data: dict): global _current_scan_id @@ -31,8 +54,9 @@ def broadcast(event: str, data: dict): q.put_nowait(msg) except queue.Full: pass - # Clear scan_id after scan_done so replay knows the scan is finished - if event == "scan_done" and _current_scan_id: + # Clear scan_id when the scan ends so replay knows it is finished + if event in _TERMINAL_EVENTS and _current_scan_id: + _finished_scan_ids.append(_current_scan_id) _current_scan_id = "" # When no browser is watching (e.g. scheduled scan), log key events if not _sse_queues: diff --git a/static/js/CLAUDE.md b/static/js/CLAUDE.md index 058671e..217e470 100644 --- a/static/js/CLAUDE.md +++ b/static/js/CLAUDE.md @@ -34,6 +34,11 @@ Never revert to `!!window._googleConnected` / `_fileSources.length > 0` — thos - **Do not close `S.es` in `scan_done` if other scans are still running** — M365 (`scan_done`), Google (`google_scan_done`), and File (`file_scan_done`) each emit their own done event. Close `S.es` only when all concurrent scans have finished: `scan_done` checks `!S._googleScanRunning && !S._fileScanRunning`; `google_scan_done` checks `!S._m365ScanRunning && !S._fileScanRunning`; `file_scan_done` checks `!S._m365ScanRunning && !S._googleScanRunning`. - **Scheduled scans** — `S._userStartedScan` is false for scheduler-triggered runs, so SSE is never closed and future scheduler events continue to arrive. - **Two separate abort events** — `state._scan_abort` (M365 + file) and `state._google_scan_abort` (Google). `POST /api/scan/stop` sets **both**. `_check_abort()` inside `_run_google_scan` must use the module-level `_scan_abort` alias (`= state._google_scan_abort`), not `gdpr_scanner._scan_abort`. +- **Stop must never look like a restart** — four rules, each fixing a way Stop used to appear to restart the scan: + - `sse.replay_messages()` never replays a scan that has ended. `scan_done`, `scan_cancelled` and `file_scan_done` move `_current_scan_id` into `_finished_scan_ids`. Do not go back to clearing the id only on `scan_done`: stopped M365 scans and all file scans stayed "current", and every reconnect (page load, watchdog) replayed them, bringing back the Stop button and progress bar. + - The `scan_cancelled` handler clears `S._m365ScanRunning` and, like `scan_done`, keeps the Stop button and SSE only while a Google or file scan is still running. `scheduler_done` and `scheduler_error` clear all three running flags. + - The Stop button goes through `_setStopBtnDisplay(display, fresh)`; do not set `stopBtn.style.display` directly. `stopScan()` switches it to a disabled "Stopping…" state. Hiding it, or passing `fresh=true` (from `startScan()` and `scheduler_started`), resets it. + - Server side: `run_scan` checks for Stop straight after collection, before `scan_start`. The scheduler checks `_scan_abort` between stages and never clears the Google abort flag after a Stop. A stopped job is recorded as `cancelled`, with no email or retention. - **`_check_abort()` emits `google_scan_done`, not `scan_cancelled`** — `scan_cancelled` unconditionally closes the SSE; `google_scan_done` checks whether other scans are still running before closing. - **`scan_phase` replay sets running flags — handled by `sse_replay_done`** — the `scan_phase` handler sets running flags to `true` whenever all flags are `false` and a source keyword is found in the phase text. On page refresh this fires during SSE replay of a completed scan, temporarily making the scan appear running. The `sse_replay_done` handler retries `loadHistorySession(null)` if no scan is running and `S._historyRefScanId` is still `null` after replay. Do not remove either the flag-setting logic or the retry. - **Google Drive uses a lazy generator, not `list()`** — `iter_drive_files()` iterated directly so `_check_abort()` fires between items. Wrapping in `list()` blocks the thread for the entire enumeration. diff --git a/static/js/results.js b/static/js/results.js index b13d0de..c2eb1ff 100644 --- a/static/js/results.js +++ b/static/js/results.js @@ -779,7 +779,7 @@ function _sseWatchdog() { _ensureSSE(); if (status.running && !S._m365ScanRunning && !S._googleScanRunning && !S._fileScanRunning) { document.getElementById('scanBtn').disabled = true; - document.getElementById('stopBtn').style.display = 'inline-block'; + _setStopBtnDisplay('inline-block'); // status.running reflects the M365 + file lock; treat as an M365 reconnect S._m365ScanRunning = true; _renderProgressSegments(); document.getElementById('progressFile').textContent = t('m365_sse_reconnecting', 'Reconnecting to running scan…'); diff --git a/static/js/scan.js b/static/js/scan.js index 5e9b4db..817ee23 100644 --- a/static/js/scan.js +++ b/static/js/scan.js @@ -317,7 +317,7 @@ function _attachScanListeners(source) { else if (phaseSrc === 'file') { S._fileScanRunning = true; } else { S._m365ScanRunning = true; } document.getElementById('scanBtn').disabled = true; - document.getElementById('stopBtn').style.display = 'inline-block'; + _setStopBtnDisplay('inline-block'); _renderProgressSegments(); } _setProgressPhase(d.phase); @@ -330,9 +330,9 @@ function _attachScanListeners(source) { : (d.total > 0 ? Math.round((d.index || d.completed || 0) / d.total * 100) : 0); S._srcPct[src] = pct; // If reconnecting mid-scan the running flag may not be set yet — ensure segment exists - if (src === 'm365' && !S._m365ScanRunning) { S._m365ScanRunning = true; document.getElementById('scanBtn').disabled = true; document.getElementById('stopBtn').style.display = 'inline-block'; _renderProgressSegments(); } - if (src === 'google' && !S._googleScanRunning) { S._googleScanRunning = true; document.getElementById('scanBtn').disabled = true; document.getElementById('stopBtn').style.display = 'inline-block'; _renderProgressSegments(); } - if (src === 'file' && !S._fileScanRunning) { S._fileScanRunning = true; document.getElementById('scanBtn').disabled = true; document.getElementById('stopBtn').style.display = 'inline-block'; _renderProgressSegments(); } + if (src === 'm365' && !S._m365ScanRunning) { S._m365ScanRunning = true; document.getElementById('scanBtn').disabled = true; _setStopBtnDisplay('inline-block'); _renderProgressSegments(); } + if (src === 'google' && !S._googleScanRunning) { S._googleScanRunning = true; document.getElementById('scanBtn').disabled = true; _setStopBtnDisplay('inline-block'); _renderProgressSegments(); } + if (src === 'file' && !S._fileScanRunning) { S._fileScanRunning = true; document.getElementById('scanBtn').disabled = true; _setStopBtnDisplay('inline-block'); _renderProgressSegments(); } var fill = document.getElementById('progressFill_' + src); if (fill) fill.style.width = pct + '%'; document.getElementById('progressFile').textContent = d.file || ''; @@ -368,15 +368,20 @@ function _attachScanListeners(source) { log((d.file ? d.file + ': ' : '') + d.error, 'err'); }); source.addEventListener('scan_cancelled', function() { - if (S._userStartedScan) { + // scan_cancelled is the M365 engine's "stopped" event. Clear its running + // flag (leaving it set made later replays/status polls treat the stopped + // scan as live) and, like scan_done, only tear down the UI once no Google + // or file scan is still running. + S._m365ScanRunning = false; + var _anyRunning = S._googleScanRunning || S._fileScanRunning; + if (S._userStartedScan && !_anyRunning) { S._userStartedScan = false; if (S.es) { S.es.close(); S.es = null; } } - document.getElementById('scanBtn').disabled = false; - document.getElementById('stopBtn').style.display = 'none'; - _clearProgressBar(); - setLogLive(''); - log('Scan stopped.', 'warn'); + document.getElementById('scanBtn').disabled = _anyRunning; + _setStopBtnDisplay(_anyRunning ? 'inline-block' : 'none'); + if (_anyRunning) { _renderProgressSegments(); } else { _clearProgressBar(); setLogLive(''); } + log(t('m365_scan_stopped', 'Scan stopped.'), 'warn'); }); source.addEventListener('scan_done', function(e) { var d = JSON.parse(e.data); @@ -401,7 +406,7 @@ function _attachScanListeners(source) { } if (!_anyRunning) setLogLive(''); document.getElementById('scanBtn').disabled = _anyRunning; - document.getElementById('stopBtn').style.display = _anyRunning ? 'inline-block' : 'none'; + _setStopBtnDisplay(_anyRunning ? 'inline-block' : 'none'); if (!_anyRunning) _clearProgressBar(); document.getElementById('statsSection').style.display = 'block'; document.getElementById('statScanned').textContent = d.total_scanned; @@ -437,7 +442,7 @@ function _attachScanListeners(source) { } setLogLive(''); document.getElementById('scanBtn').disabled = false; - document.getElementById('stopBtn').style.display = 'none'; + _setStopBtnDisplay('none'); _clearProgressBar(); document.getElementById('statsSection').style.display = 'block'; document.getElementById('statsPill').style.display = 'block'; @@ -466,7 +471,7 @@ function _attachScanListeners(source) { } setLogLive(''); document.getElementById('scanBtn').disabled = false; - document.getElementById('stopBtn').style.display = 'none'; + _setStopBtnDisplay('none'); _clearProgressBar(); document.getElementById('statsSection').style.display = 'block'; document.getElementById('statsPill').style.display = 'block'; @@ -502,7 +507,7 @@ function _attachSchedulerListeners(source) { log('\uD83D\uDD50 ' + t('m365_sched_title','Scheduled scan') + ': ' + (d.job_name||'') + '\u2026'); // Show progress UI so scan_phase / scan_progress events are visible document.getElementById('scanBtn').disabled = true; - document.getElementById('stopBtn').style.display = 'inline-block'; + _setStopBtnDisplay('inline-block', true); S._srcPct = { m365: 0, google: 0, file: 0 }; S._m365ScanRunning = true; _renderProgressSegments(); _setProgressPhase((d.job_name||'') + '\u2026'); document.getElementById('progressFile').textContent = ''; @@ -512,25 +517,32 @@ function _attachSchedulerListeners(source) { // scheduler_started was missed (e.g. browser reconnected mid-scan) console.log('[SSE] scan_start received'); document.getElementById('scanBtn').disabled = true; - document.getElementById('stopBtn').style.display = 'inline-block'; + _setStopBtnDisplay('inline-block'); // Ensure at least the M365 segment is rendered (scan_start is M365-only) if (!S._m365ScanRunning) { S._m365ScanRunning = true; _renderProgressSegments(); } }); source.addEventListener('scheduler_done', function(e) { var d = JSON.parse(e.data); console.log('[SSE] scheduler_done received:', d); + // The job's stages are all over — scheduler_started set the M365 flag + S._m365ScanRunning = S._googleScanRunning = S._fileScanRunning = false; document.getElementById('scanBtn').disabled = false; - document.getElementById('stopBtn').style.display = 'none'; + _setStopBtnDisplay('none'); _clearProgressBar(); - log('\u2713 ' + t('m365_sched_title','Scheduled scan') + ' ' + (d.job_name||'') + ' \u2014 ' + (d.flagged||0) + ' flagged', 'ok'); + if (d.cancelled) { + log('\u23f9 ' + t('m365_sched_title','Scheduled scan') + ' ' + (d.job_name||'') + ' \u2014 ' + t('m365_sched_stopped','stopped') + ' (' + (d.flagged||0) + ' flagged)', 'warn'); + } else { + log('\u2713 ' + t('m365_sched_title','Scheduled scan') + ' ' + (d.job_name||'') + ' \u2014 ' + (d.flagged||0) + ' flagged', 'ok'); + } markOverdueCards(); loadTrend(); }); source.addEventListener('scheduler_error', function(e) { var d = JSON.parse(e.data); console.log('[SSE] scheduler_error received:', d); + S._m365ScanRunning = S._googleScanRunning = S._fileScanRunning = false; document.getElementById('scanBtn').disabled = false; - document.getElementById('stopBtn').style.display = 'none'; + _setStopBtnDisplay('none'); _clearProgressBar(); log('\u26A0 ' + t('m365_sched_title','Scheduled scan') + ' failed: ' + (d.error||''), 'err'); }); @@ -564,7 +576,7 @@ function startScan(resume) { S._progressCurrentUser = ''; _renderProgressSegments(); document.getElementById('scanBtn').disabled = true; - document.getElementById('stopBtn').style.display = 'inline-block'; + _setStopBtnDisplay('inline-block', true); // progress segments rendered by _renderProgressSegments() called above document.getElementById('progressFile').textContent = ''; _setProgressPhase(t('scan_preparing', 'Preparing…')); @@ -594,7 +606,7 @@ function startScan(resume) { if (label === 'google') S._googleScanRunning = false; if (!S._m365ScanRunning && !S._googleScanRunning && !S._fileScanRunning) { document.getElementById('scanBtn').disabled = false; - document.getElementById('stopBtn').style.display = 'none'; + _setStopBtnDisplay('none'); if (S.es) { S.es.close(); S.es = null; } S._userStartedScan = false; } @@ -664,8 +676,30 @@ function startScan(resume) { } +// Show/hide the Stop button. Hiding — or passing fresh=true when a new scan +// starts — resets it from its "Stopping…" state, so a new scan always gets a +// normal, clickable Stop button even if a previous scan never reported its end. +function _setStopBtnDisplay(display, fresh) { + const btn = document.getElementById('stopBtn'); + if (!btn) return; + btn.style.display = display; + if (display === 'none' || fresh) { + btn.disabled = false; + btn.textContent = t('m365_btn_stop', 'Stop'); + } +} + function stopScan() { - fetch('/api/scan/stop', {method:'POST'}); + // Immediate feedback: the scan only notices the stop between items, which + // can take a while (e.g. OCR on a large PDF). Stays "Stopping…" until the + // scan's done/stopped event hides the button. + const btn = document.getElementById('stopBtn'); + if (btn) { btn.disabled = true; btn.textContent = t('m365_btn_stopping', 'Stopping…'); } + log(t('m365_scan_stopping', 'Stopping scan — finishing the current item…'), 'warn'); + fetch('/api/scan/stop', {method:'POST'}).catch(function(e) { + if (btn) { btn.disabled = false; btn.textContent = t('m365_btn_stop', 'Stop'); } + log('Stop failed: ' + e, 'err'); + }); } // ── Trend sparkline (#7) ────────────────────────────────────────────────────── @@ -795,6 +829,7 @@ window._attachScanListeners = _attachScanListeners; window._attachSchedulerListeners = _attachSchedulerListeners; window.startScan = startScan; window.stopScan = stopScan; +window._setStopBtnDisplay = _setStopBtnDisplay; window.drawSparkline = drawSparkline; window.loadTrend = loadTrend; window.updateStats = updateStats; diff --git a/static/js/scheduler.js b/static/js/scheduler.js index bc7aa7e..730a85d 100644 --- a/static/js/scheduler.js +++ b/static/js/scheduler.js @@ -242,7 +242,8 @@ function schedLoadHistory() { var html = ''; runs.forEach(function(r) { var ts = r.started_at ? new Date(r.started_at * 1000).toLocaleString() : '-'; - var icon = r.status === 'completed' ? '\u2713' : r.status === 'failed' ? '\u2716' : '\u23f3'; + var icon = r.status === 'completed' ? '\u2713' : r.status === 'failed' ? '\u2716' + : r.status === 'cancelled' ? '\u23f9' : '\u23f3'; var jname = r.job_name ? '' + _esc(r.job_name) + ' - ' : ''; html += icon + ' ' + jname + ts + ' - ' + (r.flagged||0) + ' flagged'; if (r.emailed) html += ' \u2709'; diff --git a/tests/test_stop_scan.py b/tests/test_stop_scan.py new file mode 100644 index 0000000..4f29105 --- /dev/null +++ b/tests/test_stop_scan.py @@ -0,0 +1,218 @@ +""" +Stop must stop — regressions for "pressing Stop seems to restart the scan". + + - SSE replay: a stopped/finished scan's buffered events are never replayed to + a reconnecting browser (that re-showed the Stop button and progress bar). + - run_scan: Stop during collection ends the scan (checkpoint + scan_cancelled) + instead of announcing scan_start / "Scanning…". + - Scheduler: Stop during one stage prevents later stages — in particular the + Google stage no longer clears the Google abort flag and scans everyone. +""" +from __future__ import annotations +from unittest.mock import MagicMock + +import pytest + +import sse + + +@pytest.fixture(autouse=True) +def clean_scan_state(): + """Restore SSE globals and abort flags so tests don't leak into each other.""" + from routes import state + saved_buf = list(sse._sse_buffer) + saved_id = sse._current_scan_id + saved_finished = list(sse._finished_scan_ids) + sse._sse_buffer.clear() + sse._current_scan_id = "" + sse._finished_scan_ids.clear() + yield + state._scan_abort.clear() + state._google_scan_abort.clear() + sse._sse_buffer.clear() + sse._sse_buffer.extend(saved_buf) + sse._current_scan_id = saved_id + sse._finished_scan_ids.clear() + sse._finished_scan_ids.extend(saved_finished) + + +def _events(msgs: list[str]) -> list[str]: + return [m.split("\n", 1)[0].removeprefix("event: ") for m in msgs] + + +# --------------------------------------------------------------------------- +# SSE replay +# --------------------------------------------------------------------------- + +class TestReplay: + def test_running_scan_is_replayed(self): + sse._current_scan_id = "scan_1" + sse.broadcast("scan_phase", {"phase": "Collecting…"}) + sse.broadcast("scan_progress", {"pct": 10}) + assert _events(sse.replay_messages()) == ["scan_phase", "scan_progress"] + + def test_stopped_scan_is_not_replayed(self): + sse._current_scan_id = "scan_1" + sse.broadcast("scan_phase", {"phase": "Collecting…"}) + sse.broadcast("scan_start", {"total": 5}) + sse.broadcast("scan_progress", {"pct": 40}) + sse.broadcast("scan_cancelled", {"completed": 2}) + assert sse._current_scan_id == "" + assert sse.replay_messages() == [] + + def test_completed_scan_is_not_replayed(self): + sse._current_scan_id = "scan_1" + sse.broadcast("scan_progress", {"pct": 100}) + sse.broadcast("scan_done", {"total_scanned": 1, "flagged_count": 0}) + assert sse.replay_messages() == [] + + def test_file_scan_done_ends_the_file_scan(self): + # Previously only scan_done cleared the id, so every finished file scan + # stayed "current" and was replayed on each page load. + sse._current_scan_id = "filescan_1" + sse.broadcast("scan_phase", {"phase": "Files — Share"}) + sse.broadcast("file_scan_done", {"total_scanned": 3, "flagged_count": 0}) + assert sse._current_scan_id == "" + assert sse.replay_messages() == [] + + def test_untagged_events_still_replayed_after_scan_ends(self): + sse._current_scan_id = "scan_1" + sse.broadcast("scan_progress", {"pct": 40}) + sse.broadcast("scan_cancelled", {"completed": 2}) + sse.broadcast("google_scan_done", {"total_scanned": 7}) # no scan running → untagged + assert _events(sse.replay_messages()) == ["google_scan_done"] + + def test_new_scan_replays_only_its_own_events(self): + sse._current_scan_id = "scan_1" + sse.broadcast("scan_progress", {"pct": 40}) + sse.broadcast("scan_cancelled", {"completed": 2}) + sse._current_scan_id = "scan_2" + sse.broadcast("scan_phase", {"phase": "Collecting…"}) + msgs = sse.replay_messages() + assert _events(msgs) == ["scan_phase"] + assert '"scan_id": "scan_2"' in msgs[0] + + +# --------------------------------------------------------------------------- +# run_scan: Stop during collection +# --------------------------------------------------------------------------- + +class TestRunScanStopDuringCollection: + @pytest.fixture() + def engine(self, monkeypatch): + import scan_engine + from routes import state + sent: list[tuple[str, dict]] = [] + checkpoints: list = [] + monkeypatch.setattr(scan_engine, "broadcast", lambda ev, data: sent.append((ev, data))) + monkeypatch.setattr(scan_engine, "DB_OK", False) + monkeypatch.setattr(scan_engine, "_load_checkpoint", lambda key, **kw: None) + monkeypatch.setattr(scan_engine, "_save_checkpoint", lambda *a, **kw: checkpoints.append(a)) + monkeypatch.setattr(scan_engine, "_clear_checkpoint", lambda **kw: None) + conn = MagicMock() + conn.is_app_mode = True + monkeypatch.setattr(state, "connector", conn) + monkeypatch.setattr(state, "flagged_items", []) + opts = {"sources": [], "user_ids": [{"id": "u1", "displayName": "User"}], "options": {}} + return scan_engine, sent, checkpoints, opts + + def test_stop_before_processing_does_not_announce_scan_start(self, engine): + scan_engine, sent, checkpoints, opts = engine + from routes import state + state._scan_abort.set() + scan_engine.run_scan(opts) + names = [e for e, _ in sent] + assert "scan_cancelled" in names + assert "scan_start" not in names + assert not any(e == "scan_phase" and "Scanning" in d.get("phase", "") for e, d in sent) + assert "scan_done" not in names + assert len(checkpoints) == 1 # resumable + + def test_without_stop_scan_runs_to_done(self, engine): + scan_engine, sent, _, opts = engine + scan_engine.run_scan(opts) + names = [e for e, _ in sent] + assert "scan_start" in names and "scan_done" in names + assert "scan_cancelled" not in names + + +# --------------------------------------------------------------------------- +# Scheduler: Stop ends the whole job +# --------------------------------------------------------------------------- + +class TestSchedulerStop: + JOB = {"id": "j1", "name": "Nightly", "profile_id": "", "enabled": True, + "auto_email": True, "auto_retention": True, "retention_years": 5} + OPTIONS = {"sources": ["email", "gmail"], "user_ids": [], "options": {}, + "file_sources": [{"id": "f1", "path": "/srv/share", "label": "Share"}], + "google_sources": ["gmail"]} + + @pytest.fixture() + def job(self, monkeypatch): + import gdpr_scanner + import scan_scheduler + import routes.google_scan as google_scan + from routes import state + + sent: list[tuple[str, dict]] = [] + monkeypatch.setattr(scan_scheduler, "load_jobs", lambda: [dict(self.JOB)]) + monkeypatch.setattr(gdpr_scanner, "_get_db", lambda *a, **kw: None) + monkeypatch.setattr(gdpr_scanner, "broadcast", lambda ev, data: sent.append((ev, data))) + conn = MagicMock() + conn.is_authenticated.return_value = True + monkeypatch.setattr(state, "connector", conn) + monkeypatch.setattr(state, "google_connector", MagicMock()) + monkeypatch.setattr(state, "flagged_items", [{"id": "found"}]) + + mocks = { + "run_scan": MagicMock(), + "run_file_scan": MagicMock(), + "google": MagicMock(), + } + monkeypatch.setattr(gdpr_scanner, "run_scan", mocks["run_scan"]) + monkeypatch.setattr(gdpr_scanner, "run_file_scan", mocks["run_file_scan"]) + monkeypatch.setattr(google_scan, "_run_google_scan", mocks["google"]) + + sched = scan_scheduler.ScanScheduler() + monkeypatch.setattr(sched, "_build_options", lambda cfg: { + k: (list(v) if isinstance(v, list) else dict(v) if isinstance(v, dict) else v) + for k, v in self.OPTIONS.items()}) + mocks["email"] = MagicMock() + mocks["retention"] = MagicMock() + monkeypatch.setattr(sched, "_send_email_report", mocks["email"]) + monkeypatch.setattr(sched, "_run_retention", mocks["retention"]) + return sched, mocks, sent + + def test_stop_during_m365_stage_skips_file_and_google(self, job): + from routes import state + sched, mocks, sent = job + + def _user_presses_stop(opts): # what POST /api/scan/stop does mid-scan + state._scan_abort.set() + state._google_scan_abort.set() + mocks["run_scan"].side_effect = _user_presses_stop + + sched._execute_scan("j1") + + mocks["run_scan"].assert_called_once() + mocks["run_file_scan"].assert_not_called() + mocks["google"].assert_not_called() + assert state._google_scan_abort.is_set(), "Google abort must not be cleared after Stop" + run = sched._last_runs["j1"] + assert run["status"] == "cancelled" + mocks["email"].assert_not_called() + mocks["retention"].assert_not_called() + done = [d for e, d in sent if e == "scheduler_done"] + assert done and done[0]["cancelled"] is True + + def test_uninterrupted_job_runs_every_stage(self, job): + sched, mocks, sent = job + sched._execute_scan("j1") + mocks["run_scan"].assert_called_once() + mocks["run_file_scan"].assert_called_once() + mocks["google"].assert_called_once() + assert sched._last_runs["j1"]["status"] == "completed" + mocks["email"].assert_called_once() + mocks["retention"].assert_called_once() + done = [d for e, d in sent if e == "scheduler_done"] + assert done and done[0]["cancelled"] is False