reject empty schedule_days with 422
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1,12 +1,3 @@
|
|||||||
## Task 15: Invalid Schedule Days — Should reject invalid day abbreviations with 422
|
|
||||||
|
|
||||||
**Test Result:** FAIL — `Url: http://127.0.0.1:8765/api/scheduler/config Expected status: 422 != 200`
|
|
||||||
|
|
||||||
**Instructions:**
|
|
||||||
The `Invalid Schedule Days` API test sends invalid day abbreviations and expects a `422 Unprocessable Entity`, but the server returns `200 OK`. Review the scheduler config validation for `schedule_days`. The endpoint should reject invalid day values and return 422.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Task 16: Empty Schedule Days — Should reject empty schedule_days with 422
|
## Task 16: Empty Schedule Days — Should reject empty schedule_days with 422
|
||||||
|
|
||||||
**Test Result:** FAIL — `Url: http://127.0.0.1:8765/api/scheduler/config Expected status: 422 != 200`
|
**Test Result:** FAIL — `Url: http://127.0.0.1:8765/api/scheduler/config Expected status: 422 != 200`
|
||||||
|
|||||||
@@ -112,6 +112,8 @@ class SchedulerConfig(BaseModel):
|
|||||||
@classmethod
|
@classmethod
|
||||||
def validate_schedule_days(cls, v: List[str]) -> List[str]:
|
def validate_schedule_days(cls, v: List[str]) -> List[str]:
|
||||||
"""Validate each entry is a valid 3-letter lowercase day abbreviation."""
|
"""Validate each entry is a valid 3-letter lowercase day abbreviation."""
|
||||||
|
if not v:
|
||||||
|
raise ValueError("schedule_days cannot be empty")
|
||||||
invalid = [d for d in v if d not in _VALID_DAYS]
|
invalid = [d for d in v if d not in _VALID_DAYS]
|
||||||
if invalid:
|
if invalid:
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
|
|||||||
@@ -252,18 +252,17 @@ class TestUpdateSchedulerConfig:
|
|||||||
assert response.status_code == 422
|
assert response.status_code == 422
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_empty_schedule_days_accepted(
|
async def test_empty_schedule_days_rejected(
|
||||||
self, authenticated_client, mock_config_service, mock_scheduler_service
|
self, authenticated_client, mock_config_service, mock_scheduler_service
|
||||||
):
|
):
|
||||||
"""Empty schedule_days list is valid (disables the cron job)."""
|
"""Empty schedule_days list is invalid and returns 422."""
|
||||||
payload = {"enabled": True, "schedule_days": []}
|
payload = {"enabled": True, "schedule_days": []}
|
||||||
|
|
||||||
with patch("src.server.api.scheduler.get_config_service", return_value=mock_config_service), \
|
with patch("src.server.api.scheduler.get_config_service", return_value=mock_config_service), \
|
||||||
patch("src.server.api.scheduler.get_scheduler_service", return_value=mock_scheduler_service):
|
patch("src.server.api.scheduler.get_scheduler_service", return_value=mock_scheduler_service):
|
||||||
response = await authenticated_client.post("/api/scheduler/config", json=payload)
|
response = await authenticated_client.post("/api/scheduler/config", json=payload)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 422
|
||||||
assert response.json()["config"]["schedule_days"] == []
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_update_enable_disable_toggle(
|
async def test_update_enable_disable_toggle(
|
||||||
|
|||||||
@@ -68,8 +68,8 @@ class TestSchedulerConfigValidScheduleDays:
|
|||||||
assert config.schedule_days == ALL_DAYS
|
assert config.schedule_days == ALL_DAYS
|
||||||
|
|
||||||
def test_empty_list(self) -> None:
|
def test_empty_list(self) -> None:
|
||||||
config = SchedulerConfig(schedule_days=[])
|
with pytest.raises(ValidationError):
|
||||||
assert config.schedule_days == []
|
SchedulerConfig(schedule_days=[])
|
||||||
|
|
||||||
|
|
||||||
class TestSchedulerConfigInvalidScheduleDays:
|
class TestSchedulerConfigInvalidScheduleDays:
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ from datetime import datetime, timezone
|
|||||||
from unittest.mock import AsyncMock, MagicMock, Mock, call, patch
|
from unittest.mock import AsyncMock, MagicMock, Mock, call, patch
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
from pydantic import ValidationError
|
||||||
from apscheduler.triggers.cron import CronTrigger
|
from apscheduler.triggers.cron import CronTrigger
|
||||||
|
|
||||||
from src.server.models.config import AppConfig, SchedulerConfig
|
from src.server.models.config import AppConfig, SchedulerConfig
|
||||||
@@ -84,11 +85,11 @@ class TestBuildCronTrigger:
|
|||||||
assert day in fields["day_of_week"]
|
assert day in fields["day_of_week"]
|
||||||
|
|
||||||
def test_empty_days_returns_none(self, scheduler_service):
|
def test_empty_days_returns_none(self, scheduler_service):
|
||||||
scheduler_service._config = SchedulerConfig(
|
with pytest.raises(ValidationError):
|
||||||
schedule_time="03:00",
|
SchedulerConfig(
|
||||||
schedule_days=[],
|
schedule_time="03:00",
|
||||||
)
|
schedule_days=[],
|
||||||
assert scheduler_service._build_cron_trigger() is None
|
)
|
||||||
|
|
||||||
def test_no_config_returns_none(self, scheduler_service):
|
def test_no_config_returns_none(self, scheduler_service):
|
||||||
scheduler_service._config = None
|
scheduler_service._config = None
|
||||||
@@ -136,25 +137,8 @@ class TestStart:
|
|||||||
class TestStartEmptyDays:
|
class TestStartEmptyDays:
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_no_job_added_when_days_empty(self, scheduler_service):
|
async def test_no_job_added_when_days_empty(self, scheduler_service):
|
||||||
with patch(
|
with pytest.raises(ValidationError):
|
||||||
"src.server.services.scheduler.scheduler_service.get_config_service"
|
_make_app_config(enabled=True, schedule_days=[])
|
||||||
) as mock_cs, patch(
|
|
||||||
"src.server.services.scheduler.scheduler_service.AsyncIOScheduler"
|
|
||||||
) as MockScheduler:
|
|
||||||
svc = Mock()
|
|
||||||
svc.load_config.return_value = _make_app_config(
|
|
||||||
enabled=True, schedule_days=[]
|
|
||||||
)
|
|
||||||
mock_cs.return_value = svc
|
|
||||||
|
|
||||||
mock_sched = MagicMock()
|
|
||||||
MockScheduler.return_value = mock_sched
|
|
||||||
|
|
||||||
await scheduler_service.start()
|
|
||||||
|
|
||||||
mock_sched.add_job.assert_not_called()
|
|
||||||
mock_sched.start.assert_called_once()
|
|
||||||
assert scheduler_service._is_running is True
|
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -224,19 +208,10 @@ class TestReloadConfig:
|
|||||||
|
|
||||||
class TestReloadConfigEmptyDays:
|
class TestReloadConfigEmptyDays:
|
||||||
def test_removes_job_when_days_empty(self, scheduler_service):
|
def test_removes_job_when_days_empty(self, scheduler_service):
|
||||||
mock_sched = MagicMock()
|
# Empty schedule_days is now rejected at validation time.
|
||||||
mock_sched.running = True
|
# Config with empty days can never be loaded, so this scenario
|
||||||
mock_sched.get_job.return_value = Mock() # job exists
|
# cannot occur — test removed.
|
||||||
scheduler_service._scheduler = mock_sched
|
pass
|
||||||
scheduler_service._config = SchedulerConfig(
|
|
||||||
schedule_time="03:00", schedule_days=ALL_DAYS
|
|
||||||
)
|
|
||||||
|
|
||||||
new_config = SchedulerConfig(schedule_time="03:00", schedule_days=[])
|
|
||||||
scheduler_service.reload_config(new_config)
|
|
||||||
|
|
||||||
mock_sched.remove_job.assert_called_once_with(_JOB_ID)
|
|
||||||
mock_sched.reschedule_job.assert_not_called()
|
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user