From 9ba783768556227df2a10c3319b633a0f9089272 Mon Sep 17 00:00:00 2001 From: Somil450 Date: Thu, 28 May 2026 23:47:18 +0530 Subject: [PATCH 1/3] feat: Add scheduled and recurring scan support with cron expression (Issue #374) --- backend/requirements.txt | 2 + backend/secuscan/database.py | 16 +++++- backend/secuscan/routes.py | 13 ++++- backend/secuscan/workflows.py | 34 ++++++++++--- frontend/src/api.ts | 4 ++ frontend/src/pages/Workflows.tsx | 86 ++++++++++++++++++++++++++------ 6 files changed, 128 insertions(+), 27 deletions(-) diff --git a/backend/requirements.txt b/backend/requirements.txt index b7d7a851..2b9249ad 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -10,3 +10,5 @@ python-multipart>=0.0.9 xhtml2pdf>=0.2.17 aiosqlite>=0.20.0 python-whois>=0.9.4 +httpx>=0.28.1 +croniter>=2.0.0 diff --git a/backend/secuscan/database.py b/backend/secuscan/database.py index 11c65617..baf0c30a 100644 --- a/backend/secuscan/database.py +++ b/backend/secuscan/database.py @@ -166,6 +166,7 @@ async def _create_schema(self): id TEXT PRIMARY KEY, name TEXT NOT NULL UNIQUE, schedule_seconds INTEGER, + cron_expression TEXT, enabled BOOLEAN NOT NULL DEFAULT 1, steps_json TEXT NOT NULL DEFAULT '[]', created_at TIMESTAMP NOT NULL DEFAULT (datetime('now')), @@ -251,9 +252,20 @@ async def _create_schema(self): if col_name not in existing_finding_cols: try: await self.execute(f"ALTER TABLE findings ADD COLUMN {col_name} {col_type}") - print(f"Added missing column {col_name} to findings table.") + print(f"Added missing column '{col_name}' to findings table.") except Exception as e: - print(f"Failed to add column {col_name}: {e}") + print(f"Failed to add '{col_name}' to findings: {e}") + + # Workflows table migration + workflows_columns = await self.fetchall("PRAGMA table_info(workflows)") + existing_wf_cols = {col["name"] for col in workflows_columns} + if "cron_expression" not in existing_wf_cols: + try: + await self.execute("ALTER TABLE workflows ADD COLUMN cron_expression TEXT") + print("Added missing column 'cron_expression' to workflows table.") + except Exception as e: + print(f"Failed to add 'cron_expression' to workflows: {e}") + await self._backfill_risk_scores() diff --git a/backend/secuscan/routes.py b/backend/secuscan/routes.py index 1258fa94..635fe5ea 100644 --- a/backend/secuscan/routes.py +++ b/backend/secuscan/routes.py @@ -48,6 +48,7 @@ def _serialize_workflow(row: Dict[str, Any], queued_task_ids: Optional[List[str] "id": row["id"], "name": row["name"], "schedule_seconds": row.get("schedule_seconds"), + "cron_expression": row.get("cron_expression"), "enabled": bool(row.get("enabled")), "steps": _parse_workflow_steps(row.get("steps_json")), "created_at": row.get("created_at"), @@ -1032,17 +1033,21 @@ async def create_workflow(payload: Dict[str, Any]): workflow_id = str(uuid.uuid4()) schedule_seconds = payload.get("schedule_seconds") + cron_expression = payload.get("cron_expression") + if cron_expression: + cron_expression = str(cron_expression).strip() enabled = bool(payload.get("enabled", True)) db = await get_db() await db.execute( """ - INSERT INTO workflows (id, name, schedule_seconds, enabled, steps_json) - VALUES (?, ?, ?, ?, ?) + INSERT INTO workflows (id, name, schedule_seconds, cron_expression, enabled, steps_json) + VALUES (?, ?, ?, ?, ?, ?) """, ( workflow_id, name, int(schedule_seconds) if schedule_seconds else None, + cron_expression if cron_expression else None, 1 if enabled else 0, json.dumps(steps), ), @@ -1095,6 +1100,10 @@ async def update_workflow(workflow_id: str, payload: Dict[str, Any]): val = payload["schedule_seconds"] updates.append("schedule_seconds = ?") params.append(int(val) if val else None) + if "cron_expression" in payload: + cval = payload["cron_expression"] + updates.append("cron_expression = ?") + params.append(str(cval).strip() if cval else None) if "enabled" in payload: updates.append("enabled = ?") params.append(1 if payload["enabled"] else 0) diff --git a/backend/secuscan/workflows.py b/backend/secuscan/workflows.py index 940376d7..42e28357 100644 --- a/backend/secuscan/workflows.py +++ b/backend/secuscan/workflows.py @@ -5,7 +5,8 @@ import json import logging from datetime import datetime, timezone -from typing import Any, Dict, List +from typing import Any, Dict, List, Optional +from croniter import croniter from .database import get_db from .executor import executor logger = logging.getLogger(__name__) @@ -39,23 +40,24 @@ async def _run_loop(self): await asyncio.sleep(5) async def tick(self): db = await get_db() + # Fetch workflows that have EITHER a schedule_seconds OR a cron_expression rows = await db.fetchall( """ - SELECT id, name, schedule_seconds, last_run_at, steps_json + SELECT id, name, schedule_seconds, cron_expression, last_run_at, steps_json FROM workflows - WHERE enabled = 1 AND schedule_seconds IS NOT NULL AND schedule_seconds > 0 + WHERE enabled = 1 AND ((schedule_seconds IS NOT NULL AND schedule_seconds > 0) OR cron_expression IS NOT NULL) """ ) now = datetime.now(timezone.utc) for row in rows: - if not self._should_run(now, row.get("last_run_at"), int(row["schedule_seconds"])): + if not self._should_run(now, row.get("last_run_at"), row.get("schedule_seconds"), row.get("cron_expression")): continue await self._run_workflow(row["id"], json.loads(row.get("steps_json") or "[]")) await db.execute( "UPDATE workflows SET last_run_at = datetime('now') WHERE id = ?", (row["id"],), ) - def _should_run(self, now: datetime, last_run_at: str | None, schedule_seconds: int) -> bool: + def _should_run(self, now: datetime, last_run_at: str | None, schedule_seconds: Optional[int], cron_expression: Optional[str]) -> bool: if not last_run_at: return True last = datetime.fromisoformat(last_run_at.replace("Z", "+00:00")) @@ -65,8 +67,26 @@ def _should_run(self, now: datetime, last_run_at: str | None, schedule_seconds: # Treat any naive timestamp from the DB as UTC. if last.tzinfo is None: last = last.replace(tzinfo=timezone.utc) - elapsed = (now - last).total_seconds() - return elapsed >= schedule_seconds + + # 1. Cron logic (takes precedence if valid) + if cron_expression: + try: + # croniter computes the next valid time after the last_run_at time + cron = croniter(cron_expression, last) + next_run = cron.get_next(datetime) + if next_run.tzinfo is None: + next_run = next_run.replace(tzinfo=timezone.utc) + return now >= next_run + except Exception as e: + logger.error(f"Invalid cron expression '{cron_expression}': {e}") + # Fallback to schedule_seconds if cron is invalid, else return False + + # 2. Interval logic + if schedule_seconds and schedule_seconds > 0: + elapsed = (now - last).total_seconds() + return elapsed >= schedule_seconds + + return False async def _run_workflow(self, workflow_id: str, steps: List[Dict[str, Any]]): logger.info("Running workflow %s with %d step(s)", workflow_id, len(steps)) for step in steps: diff --git a/frontend/src/api.ts b/frontend/src/api.ts index ec750a5e..5ce6f1a4 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -188,6 +188,7 @@ export interface Workflow { id: string name: string schedule_seconds: number | null + cron_expression?: string | null enabled: boolean steps: WorkflowStep[] last_run_at?: string | null @@ -198,6 +199,7 @@ export interface Workflow { export interface WorkflowCreatePayload { name: string schedule_seconds?: number | null + cron_expression?: string | null enabled: boolean steps: WorkflowStep[] } @@ -205,6 +207,7 @@ export interface WorkflowCreatePayload { export interface WorkflowUpdatePayload { name?: string schedule_seconds?: number | null + cron_expression?: string | null enabled?: boolean steps?: WorkflowStep[] } @@ -237,6 +240,7 @@ function normalizeWorkflow(raw: any): Workflow { id: String(raw.id), name: String(raw.name ?? ''), schedule_seconds: parseScheduleSeconds(raw.schedule_seconds), + cron_expression: raw.cron_expression ?? null, enabled: Boolean(raw.enabled), steps: parseWorkflowSteps(raw.steps ?? raw.steps_json), last_run_at: raw.last_run_at ?? null, diff --git a/frontend/src/pages/Workflows.tsx b/frontend/src/pages/Workflows.tsx index ec9371a7..209ec941 100644 --- a/frontend/src/pages/Workflows.tsx +++ b/frontend/src/pages/Workflows.tsx @@ -34,7 +34,8 @@ function timeAgo(iso?: string | null) { return `${Math.floor(hrs / 24)}d ago` } -function formatSchedule(scheduleSeconds?: number | null) { +function formatSchedule(scheduleSeconds?: number | null, cronExpression?: string | null) { + if (cronExpression) return `Cron: ${cronExpression}` if (!scheduleSeconds || scheduleSeconds <= 0) return 'Manual' if (scheduleSeconds < 60) return `Every ${scheduleSeconds}s` @@ -101,7 +102,9 @@ interface CreateSheetProps { function CreateSheet({ onClose, onCreated }: CreateSheetProps) { const [name, setName] = useState('') + const [scheduleType, setScheduleType] = useState<'interval' | 'cron'>('interval') const [scheduleSeconds, setScheduleSeconds] = useState('3600') + const [cronExpression, setCronExpression] = useState('0 0 * * *') const [enabled, setEnabled] = useState(true) const [stepsJson, setStepsJson] = useState(JSON.stringify(emptySteps, null, 2)) const [jsonError, setJsonError] = useState(null) @@ -123,17 +126,26 @@ function CreateSheet({ onClose, onCreated }: CreateSheetProps) { const trimmedSchedule = scheduleSeconds.trim() const parsedSchedule = trimmedSchedule === '' ? null : Number(trimmedSchedule) - if ( - parsedSchedule !== null && - (!Number.isInteger(parsedSchedule) || parsedSchedule <= 0) - ) { + if (scheduleType === 'interval' && parsedSchedule !== null && (!Number.isInteger(parsedSchedule) || parsedSchedule <= 0)) { setError('Schedule must be a positive whole number of seconds') return } + const trimmedCron = cronExpression.trim() + if (scheduleType === 'cron' && trimmedCron === '') { + setError('Cron expression cannot be empty') + return + } + setLoading(true) try { - const payload: WorkflowCreatePayload = { name, schedule_seconds: parsedSchedule, enabled, steps } + const payload: WorkflowCreatePayload = { + name, + schedule_seconds: scheduleType === 'interval' ? parsedSchedule : null, + cron_expression: scheduleType === 'cron' ? trimmedCron : null, + enabled, + steps + } const created = await createWorkflow(payload) onCreated(created) } catch { @@ -165,15 +177,57 @@ function CreateSheet({ onClose, onCreated }: CreateSheetProps) { /> -
- - setScheduleSeconds(e.target.value)} - placeholder="3600" - inputMode="numeric" - className="w-full bg-charcoal-dark border-4 border-black px-4 py-3 text-sm text-silver-bright font-mono placeholder:text-silver/30 focus:outline-none focus:border-rag-red transition-colors" - /> +
+
+ + +
+ + {scheduleType === 'interval' ? ( +
+ + setScheduleSeconds(e.target.value)} + placeholder="3600" + inputMode="numeric" + className="w-full bg-charcoal-dark border-4 border-black px-4 py-3 text-sm text-silver-bright font-mono placeholder:text-silver/30 focus:outline-none focus:border-rag-red transition-colors" + /> +
+ ) : ( +
+ + setCronExpression(e.target.value)} + placeholder="0 0 * * *" + className="w-full bg-charcoal-dark border-4 border-black px-4 py-3 text-sm text-silver-bright font-mono placeholder:text-silver/30 focus:outline-none focus:border-rag-red transition-colors" + /> +

+ e.g., "0 0 * * *" (daily at midnight), "0 * * * *" (hourly) +

+
+ )}
@@ -243,7 +297,7 @@ function WorkflowCard({ workflow, onToggle, onRun, onDelete, running, toggling }

{workflow.name}

-

{formatSchedule(workflow.schedule_seconds)}

+

{formatSchedule(workflow.schedule_seconds, workflow.cron_expression)}

{workflow.enabled ? 'Enabled' : 'Disabled'} From 9e53d986aa14751d9ce0fecc52b8fe348164cbbf Mon Sep 17 00:00:00 2001 From: Somil450 Date: Fri, 29 May 2026 01:57:51 +0530 Subject: [PATCH 2/3] fix: resolve failing tests and decoding errors in CI --- backend/secuscan/plugins.py | 2 +- frontend/testing/unit/api.workflows.test.ts | 1 + .../testing/unit/pages/Workflows.test.tsx | 1 + .../integration/test_database_indexes.py | 28 ++++++++++++------- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/backend/secuscan/plugins.py b/backend/secuscan/plugins.py index 45285a3d..ad10cd98 100644 --- a/backend/secuscan/plugins.py +++ b/backend/secuscan/plugins.py @@ -68,7 +68,7 @@ async def load_plugins(self) -> int: async def _load_plugin_metadata(self, metadata_file: Path) -> PluginMetadata: """Load and parse plugin metadata JSON""" - with open(metadata_file, 'r') as f: + with open(metadata_file, 'r', encoding='utf-8') as f: data = json.load(f) return PluginMetadata(**data) diff --git a/frontend/testing/unit/api.workflows.test.ts b/frontend/testing/unit/api.workflows.test.ts index 17d504ce..7793c6b7 100644 --- a/frontend/testing/unit/api.workflows.test.ts +++ b/frontend/testing/unit/api.workflows.test.ts @@ -35,6 +35,7 @@ describe('workflow API helpers', () => { id: 'wf-001', name: 'Nightly Scan', schedule_seconds: 3600, + cron_expression: null, enabled: true, steps: [{ plugin_id: 'nmap', inputs: {} }], last_run_at: null, diff --git a/frontend/testing/unit/pages/Workflows.test.tsx b/frontend/testing/unit/pages/Workflows.test.tsx index 7c304da8..0569b4c7 100644 --- a/frontend/testing/unit/pages/Workflows.test.tsx +++ b/frontend/testing/unit/pages/Workflows.test.tsx @@ -129,6 +129,7 @@ describe('Workflows — create action', () => { expect(vi.mocked(createWorkflow)).toHaveBeenCalledWith({ name: 'Nightly Scan', schedule_seconds: 7200, + cron_expression: null, enabled: true, steps: [{ plugin_id: '', inputs: {} }], }) diff --git a/testing/backend/integration/test_database_indexes.py b/testing/backend/integration/test_database_indexes.py index 42ffdde6..e2dd0e76 100644 --- a/testing/backend/integration/test_database_indexes.py +++ b/testing/backend/integration/test_database_indexes.py @@ -19,6 +19,14 @@ from backend.secuscan.config import settings from backend.secuscan.database import init_db +async def _init_db_safe(path): + await init_db(path) + from backend.secuscan import database + if database.db: + await database.db.disconnect() + database.db = None + + # ── Helpers ─────────────────────────────────────────────────────────────────── @@ -68,7 +76,7 @@ class TestDatabaseIndexes: def test_findings_severity_index_exists(self, setup_test_environment): """idx_findings_severity must exist for GROUP BY severity queries.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_findings_severity" in indexes, ( "Missing idx_findings_severity — dashboard GROUP BY severity will do a full scan" @@ -76,7 +84,7 @@ def test_findings_severity_index_exists(self, setup_test_environment): def test_findings_discovered_at_index_exists(self, setup_test_environment): """idx_findings_discovered_at must exist for ORDER BY discovered_at DESC.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_findings_discovered_at" in indexes, ( "Missing idx_findings_discovered_at — findings list ORDER BY will do a full scan" @@ -84,49 +92,49 @@ def test_findings_discovered_at_index_exists(self, setup_test_environment): def test_findings_task_id_index_exists(self, setup_test_environment): """idx_findings_task_id must exist for foreign key lookups.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_findings_task_id" in indexes def test_findings_task_severity_composite_index_exists(self, setup_test_environment): """idx_findings_task_severity composite index must exist.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_findings_task_severity" in indexes def test_reports_generated_at_index_exists(self, setup_test_environment): """idx_reports_generated_at must exist for reports list ORDER BY.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_reports_generated_at" in indexes def test_reports_task_id_index_exists(self, setup_test_environment): """idx_reports_task_id must exist for foreign key lookups.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_reports_task_id" in indexes def test_reports_status_index_exists(self, setup_test_environment): """idx_reports_status must exist for status filter queries.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_reports_status" in indexes def test_audit_log_timestamp_index_exists(self, setup_test_environment): """idx_audit_timestamp must exist for audit log ORDER BY timestamp.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_audit_timestamp" in indexes def test_audit_log_event_type_index_exists(self, setup_test_environment): """idx_audit_event_type must exist for event_type filter queries.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_audit_event_type" in indexes def test_tasks_status_created_composite_index_exists(self, setup_test_environment): """idx_tasks_status_created composite index must exist.""" - asyncio.run(init_db(settings.database_path)) + asyncio.run(_init_db_safe(settings.database_path)) indexes = get_index_names(settings.database_path) assert "idx_tasks_status_created" in indexes From eeed66908f62a4fc8ae58f38803c8f805b4f480a Mon Sep 17 00:00:00 2001 From: Somil450 Date: Fri, 29 May 2026 02:09:25 +0530 Subject: [PATCH 3/3] fix: add cron scheduling tests and remove trailing whitespace --- backend/secuscan/database.py | 2 +- backend/secuscan/workflows.py | 6 +- frontend/src/pages/Workflows.tsx | 10 +- .../backend/unit/test_workflows_scheduler.py | 110 +++++++++++++++--- 4 files changed, 106 insertions(+), 22 deletions(-) diff --git a/backend/secuscan/database.py b/backend/secuscan/database.py index baf0c30a..ac4f1c12 100644 --- a/backend/secuscan/database.py +++ b/backend/secuscan/database.py @@ -255,7 +255,7 @@ async def _create_schema(self): print(f"Added missing column '{col_name}' to findings table.") except Exception as e: print(f"Failed to add '{col_name}' to findings: {e}") - + # Workflows table migration workflows_columns = await self.fetchall("PRAGMA table_info(workflows)") existing_wf_cols = {col["name"] for col in workflows_columns} diff --git a/backend/secuscan/workflows.py b/backend/secuscan/workflows.py index 42e28357..3eb002f6 100644 --- a/backend/secuscan/workflows.py +++ b/backend/secuscan/workflows.py @@ -67,7 +67,7 @@ def _should_run(self, now: datetime, last_run_at: str | None, schedule_seconds: # Treat any naive timestamp from the DB as UTC. if last.tzinfo is None: last = last.replace(tzinfo=timezone.utc) - + # 1. Cron logic (takes precedence if valid) if cron_expression: try: @@ -80,12 +80,12 @@ def _should_run(self, now: datetime, last_run_at: str | None, schedule_seconds: except Exception as e: logger.error(f"Invalid cron expression '{cron_expression}': {e}") # Fallback to schedule_seconds if cron is invalid, else return False - + # 2. Interval logic if schedule_seconds and schedule_seconds > 0: elapsed = (now - last).total_seconds() return elapsed >= schedule_seconds - + return False async def _run_workflow(self, workflow_id: str, steps: List[Dict[str, Any]]): logger.info("Running workflow %s with %d step(s)", workflow_id, len(steps)) diff --git a/frontend/src/pages/Workflows.tsx b/frontend/src/pages/Workflows.tsx index 209ec941..ebdb069f 100644 --- a/frontend/src/pages/Workflows.tsx +++ b/frontend/src/pages/Workflows.tsx @@ -139,12 +139,12 @@ function CreateSheet({ onClose, onCreated }: CreateSheetProps) { setLoading(true) try { - const payload: WorkflowCreatePayload = { - name, - schedule_seconds: scheduleType === 'interval' ? parsedSchedule : null, + const payload: WorkflowCreatePayload = { + name, + schedule_seconds: scheduleType === 'interval' ? parsedSchedule : null, cron_expression: scheduleType === 'cron' ? trimmedCron : null, - enabled, - steps + enabled, + steps } const created = await createWorkflow(payload) onCreated(created) diff --git a/testing/backend/unit/test_workflows_scheduler.py b/testing/backend/unit/test_workflows_scheduler.py index e8236f52..10b4f3ba 100644 --- a/testing/backend/unit/test_workflows_scheduler.py +++ b/testing/backend/unit/test_workflows_scheduler.py @@ -1,12 +1,18 @@ """ Tests for WorkflowScheduler._should_run() -Covers the timezone-naive/aware datetime bug where SQLite's datetime('now') -produces strings without a timezone suffix, causing TypeError on subtraction. +Covers: +- Basic interval scheduling (schedule_seconds) +- Timezone-naive/aware datetime bug where SQLite's datetime('now') + produces strings without a timezone suffix, causing TypeError on subtraction. +- Cron expression scheduling behaviour +- Invalid cron expression handling (must log error and not raise) +- Backend validation: croniter.is_valid() rejects bad expressions """ from datetime import datetime, timezone, timedelta import pytest +from croniter import croniter from backend.secuscan.workflows import WorkflowScheduler @@ -21,30 +27,35 @@ def _now(): # --------------------------------------------------------------------------- -# Core behaviour +# Core interval behaviour (schedule_seconds, no cron) # --------------------------------------------------------------------------- def test_should_run_when_no_last_run(scheduler): """First-ever run: last_run_at is None → always run.""" - assert scheduler._should_run(_now(), None, 3600) is True + assert scheduler._should_run(_now(), None, 3600, None) is True def test_should_run_when_elapsed_exceeds_schedule(scheduler): """Last run was longer ago than schedule_seconds → run.""" last = (_now() - timedelta(seconds=7200)).isoformat() - assert scheduler._should_run(_now(), last, 3600) is True + assert scheduler._should_run(_now(), last, 3600, None) is True def test_should_not_run_when_elapsed_below_schedule(scheduler): """Last run was recent → do not run.""" last = (_now() - timedelta(seconds=60)).isoformat() - assert scheduler._should_run(_now(), last, 3600) is False + assert scheduler._should_run(_now(), last, 3600, None) is False def test_should_run_at_exact_boundary(scheduler): """Exactly at schedule_seconds elapsed → run.""" last = (_now() - timedelta(seconds=3600)).isoformat() - assert scheduler._should_run(_now(), last, 3600) is True + assert scheduler._should_run(_now(), last, 3600, None) is True + + +def test_empty_string_treated_as_no_last_run(scheduler): + """Empty string last_run_at should behave like None → run.""" + assert scheduler._should_run(_now(), "", 3600, None) is True # --------------------------------------------------------------------------- @@ -63,7 +74,7 @@ def test_sqlite_naive_datetime_does_not_raise(scheduler): # Must not raise TypeError try: - result = scheduler._should_run(now, sqlite_format, 3600) + result = scheduler._should_run(now, sqlite_format, 3600, None) assert isinstance(result, bool) except TypeError as e: pytest.fail( @@ -75,15 +86,88 @@ def test_sqlite_naive_datetime_does_not_raise(scheduler): def test_z_suffix_still_works(scheduler): """ISO strings ending in Z (UTC marker) must still be handled correctly.""" last = (_now() - timedelta(seconds=7200)).strftime("%Y-%m-%dT%H:%M:%SZ") - assert scheduler._should_run(_now(), last, 3600) is True + assert scheduler._should_run(_now(), last, 3600, None) is True def test_offset_aware_iso_string_still_works(scheduler): """Full ISO strings with +00:00 suffix must still be handled correctly.""" last = (_now() - timedelta(seconds=7200)).isoformat() - assert scheduler._should_run(_now(), last, 3600) is True + assert scheduler._should_run(_now(), last, 3600, None) is True -def test_empty_string_treated_as_no_last_run(scheduler): - """Empty string last_run_at should behave like None → run.""" - assert scheduler._should_run(_now(), "", 3600) is True \ No newline at end of file +# --------------------------------------------------------------------------- +# Cron scheduling behaviour +# --------------------------------------------------------------------------- + +def test_cron_triggers_when_past_next_run(scheduler): + """Valid cron — now is past the next scheduled time → should run.""" + # Last ran 2 hours ago; cron fires every hour → should run now + last = (_now() - timedelta(hours=2)).isoformat() + assert scheduler._should_run(_now(), None, None, "0 * * * *") is True + + +def test_cron_no_trigger_when_before_next_run(scheduler): + """Valid cron — next fire time is in the future → should NOT run yet.""" + # Last ran 30 seconds ago; cron fires every hour → too soon + last = (_now() - timedelta(seconds=30)).isoformat() + assert scheduler._should_run(_now(), last, None, "0 * * * *") is False + + +def test_cron_first_run_no_last_run_at(scheduler): + """Cron with no previous run should always trigger (same as interval).""" + assert scheduler._should_run(_now(), None, None, "*/5 * * * *") is True + + +def test_cron_takes_precedence_over_schedule_seconds(scheduler): + """When both cron and schedule_seconds are present, cron wins.""" + # Ran 30 seconds ago. Cron fires hourly → too soon. But schedule_seconds=60 + # would say run (elapsed > 60? no, 30s). Either way cron should decide. + last = (_now() - timedelta(seconds=30)).isoformat() + # cron says don't run yet; schedule_seconds=10 (would say run but cron wins) + result = scheduler._should_run(_now(), last, 10, "0 * * * *") + assert result is False # cron says not yet + + +# --------------------------------------------------------------------------- +# Invalid cron expression — must not raise, must fall back gracefully +# --------------------------------------------------------------------------- + +def test_invalid_cron_expression_does_not_raise(scheduler): + """ + An invalid cron expression stored in the DB should log an error + and fall back to schedule_seconds logic — never raise an exception. + """ + last = (_now() - timedelta(seconds=7200)).isoformat() + try: + result = scheduler._should_run(_now(), last, 3600, "NOT_A_CRON") + # Should fall back to schedule_seconds=3600 and return True (elapsed >= 3600) + assert isinstance(result, bool) + except Exception as e: + pytest.fail(f"_should_run raised {type(e).__name__} on invalid cron: {e}") + + +def test_invalid_cron_with_no_schedule_seconds_returns_false(scheduler): + """Invalid cron + no schedule_seconds → graceful False, no exception.""" + last = (_now() - timedelta(seconds=100)).isoformat() + result = scheduler._should_run(_now(), last, None, "bad-expr") + assert result is False + + +# --------------------------------------------------------------------------- +# Backend validation: croniter.is_valid() must reject bad expressions +# --------------------------------------------------------------------------- + +def test_croniter_rejects_invalid_expression(): + """croniter.is_valid() must return False for obviously invalid strings.""" + assert croniter.is_valid("NOT_A_CRON") is False + assert croniter.is_valid("bad-expr") is False + assert croniter.is_valid("99 99 99 99 99") is False + + +def test_croniter_accepts_valid_expressions(): + """croniter.is_valid() must return True for standard cron expressions.""" + assert croniter.is_valid("* * * * *") is True + assert croniter.is_valid("0 * * * *") is True + assert croniter.is_valid("*/5 * * * *") is True + assert croniter.is_valid("0 9 * * 1-5") is True + assert croniter.is_valid("30 6 1 * *") is True \ No newline at end of file