Skip to content

Commit 4b065e4

Browse files
committed
fix: validate timezone through CronTrigger model to prevent None from reaching ZoneInfo
When clients send {"timezone": null}, the raw dictionary value is None, which would cause ZoneInfo(None) to raise an exception. Instead, validate each trigger.value through CronTrigger.model_validate() to normalize None to "UTC" via the field validator. This ensures: - None timezone values are normalized to "UTC" before ZoneInfo creation - No None values are persisted to the database - Invalid timezones are caught during validation - All timezone handling uses the validated, normalized value Added test to verify null timezone normalization behavior. Signed-off-by: Sparsh <sparsh.raj30@gmail.com>
1 parent 770eb4e commit 4b065e4

3 files changed

Lines changed: 41 additions & 12 deletions

File tree

state-manager/app/tasks/verify_graph.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,19 +102,20 @@ async def verify_inputs(graph_template: GraphTemplate, registered_nodes: list[Re
102102
return errors
103103

104104
async def create_crons(graph_template: GraphTemplate):
105-
# Build a map of (expression, timezone) -> trigger for deduplication
105+
# Build a map of (expression, timezone) -> validated CronTrigger for deduplication
106106
triggers_to_create = {}
107107
for trigger in graph_template.triggers:
108108
if trigger.type == TriggerTypeEnum.CRON:
109-
expression = trigger.value["expression"]
110-
timezone = trigger.value.get("timezone", "UTC")
111-
triggers_to_create[(expression, timezone)] = trigger
109+
# Validate through CronTrigger model to normalize timezone (None -> "UTC")
110+
from app.models.trigger_models import CronTrigger
111+
cron_trigger = CronTrigger.model_validate(trigger.value)
112+
triggers_to_create[(cron_trigger.expression, cron_trigger.timezone)] = cron_trigger
112113

113114
current_time = datetime.now(ZoneInfo("UTC")).replace(tzinfo=None)
114115

115116
new_db_triggers = []
116-
for (expression, timezone), trigger in triggers_to_create.items():
117-
# Use the trigger's timezone, defaulting to UTC
117+
for (expression, timezone), cron_trigger in triggers_to_create.items():
118+
# Use the validated timezone (guaranteed to be valid IANA timezone, never None)
118119
tz = ZoneInfo(timezone)
119120

120121
# Get current time in the specified timezone
@@ -130,8 +131,8 @@ async def create_crons(graph_template: GraphTemplate):
130131
new_db_triggers.append(
131132
DatabaseTriggers(
132133
type=TriggerTypeEnum.CRON,
133-
expression=expression,
134-
timezone=timezone,
134+
expression=cron_trigger.expression,
135+
timezone=cron_trigger.timezone,
135136
graph_name=graph_template.name,
136137
namespace=graph_template.namespace,
137138
trigger_status=TriggerStatusEnum.PENDING,

state-manager/tests/unit/models/test_trigger_models.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ def test_invalid_cron_expression(self):
4343

4444
errors = exc_info.value.errors()
4545
assert len(errors) == 1
46-
assert "Invalid cron expression" in str(errors[0]["ctx"]["error"])
46+
# Check the error message (Pydantic v2 doesn't always populate ctx)
47+
error_msg = errors[0].get("msg") or str(errors[0])
48+
assert "Invalid cron expression" in error_msg
4749

4850
def test_invalid_timezone(self):
4951
"""Test creating a cron trigger with invalid timezone"""
@@ -52,8 +54,10 @@ def test_invalid_timezone(self):
5254

5355
errors = exc_info.value.errors()
5456
assert len(errors) == 1
55-
assert "Invalid timezone" in str(errors[0]["ctx"]["error"])
56-
assert "Invalid/Timezone" in str(errors[0]["ctx"]["error"])
57+
# Check the error message (Pydantic v2 doesn't always populate ctx)
58+
error_msg = errors[0].get("msg") or str(errors[0])
59+
assert "Invalid timezone" in error_msg
60+
assert "Invalid/Timezone" in error_msg
5761

5862
def test_none_timezone_defaults_to_utc(self):
5963
"""Test that None timezone defaults to UTC"""

state-manager/tests/unit/tasks/test_create_crons.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,4 +233,28 @@ async def test_create_crons_trigger_time_is_datetime():
233233

234234
# Verify trigger_time is a datetime object
235235
call_kwargs = mock_db_class.call_args[1]
236-
assert isinstance(call_kwargs['trigger_time'], datetime)
236+
assert isinstance(call_kwargs['trigger_time'], datetime)
237+
238+
@pytest.mark.asyncio
239+
async def test_create_crons_with_null_timezone_normalizes_to_utc():
240+
"""Test create_crons with null/None timezone normalizes to UTC via model validation"""
241+
graph_template = MagicMock()
242+
graph_template.name = "test_graph"
243+
graph_template.namespace = "test_ns"
244+
graph_template.triggers = [
245+
Trigger(
246+
type=TriggerTypeEnum.CRON,
247+
value={"expression": "0 9 * * *", "timezone": None} # Explicit None
248+
)
249+
]
250+
251+
with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class:
252+
mock_db_class.return_value = MagicMock()
253+
mock_db_class.insert_many = AsyncMock()
254+
255+
await create_crons(graph_template)
256+
257+
# Verify timezone was normalized to "UTC" (not None)
258+
call_kwargs = mock_db_class.call_args[1]
259+
assert call_kwargs['timezone'] == "UTC"
260+
assert call_kwargs['timezone'] is not None

0 commit comments

Comments
 (0)