Skip to content

Commit a9df29c

Browse files
Validate programmatic workflow handlers at every admission surface
1 parent 7c5b0bb commit a9df29c

5 files changed

Lines changed: 61 additions & 33 deletions

File tree

‎docs/sdk-reference.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,8 @@ Within a workflow class, signal, query, update and validator names are checked
421421
independently. One name may be shared across these kinds. Python's normal
422422
method resolution applies to inheritance: overriding the same Python attribute
423423
is allowed, while two resolved attributes with the same handler kind and wire
424-
name are rejected at decoration time. The test environment rejects a different
424+
name are rejected at decoration time and at Worker, Replayer and test-environment
425+
admission, including programmatically defined classes. The test environment rejects a different
425426
workflow class replacing an existing name, while repeated registration of the
426427
same class remains idempotent for automatic execution. Explicit activity mock
427428
and canned-result setters continue to support replacement.

‎src/durable_workflow/testing.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ def test_my_workflow():
5252
StartTimer,
5353
UpsertSearchAttributes,
5454
_validate_workflow_entrypoint,
55+
_workflow_handler_registries,
5556
registry,
5657
replay,
5758
)
@@ -136,6 +137,7 @@ def register_workflow(self, workflow_cls: type) -> None:
136137
"decorate it with @workflow.defn(name=...) first."
137138
)
138139
_validate_workflow_entrypoint(workflow_cls)
140+
_workflow_handler_registries(workflow_cls)
139141
if name in self._registered_workflows and self._registered_workflows[name] is not workflow_cls:
140142
raise DuplicateRegistrationError("workflow", name, self._registered_workflows[name], workflow_cls)
141143
self._registered_workflows[name] = workflow_cls

‎src/durable_workflow/worker.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
StartChildWorkflow,
108108
UpsertMemo,
109109
_validate_workflow_entrypoint,
110+
_workflow_handler_registries,
110111
apply_update,
111112
commands_to_server_commands,
112113
query_state,
@@ -1029,6 +1030,7 @@ def __init__(
10291030
self.workflows: dict[str, type] = {}
10301031
for definition in workflows:
10311032
_validate_workflow_entrypoint(definition)
1033+
_workflow_handler_registries(definition)
10321034
name = _workflow_name(definition)
10331035
if name in self.workflows:
10341036
raise DuplicateRegistrationError("workflow", name, self.workflows[name], definition)

‎src/durable_workflow/workflow.py‎

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,29 @@ def _validate_workflow_result(value: Any, cls: type) -> None:
125125
)
126126

127127

128+
def _workflow_handler_registries(
129+
cls: type,
130+
) -> tuple[dict[str, str], dict[str, str], dict[str, str], dict[str, str]]:
131+
handlers: dict[str, dict[str, str]] = {kind: {} for kind in ("signal", "query", "update", "update_validator")}
132+
for attr in dir(cls):
133+
if attr.startswith("_"):
134+
continue
135+
member = getattr(cls, attr, None)
136+
for kind, registry in handlers.items():
137+
wire_name = getattr(member, f"__{kind}_name__", None)
138+
if not isinstance(wire_name, str) or not wire_name:
139+
continue
140+
if kind == "signal" and wire_name == MESSAGE_STREAM_SIGNAL:
141+
raise ValueError(f"signal name {wire_name!r} is reserved by the workflow runtime")
142+
if wire_name in registry:
143+
owner = f"{cls.__module__}.{cls.__qualname__}"
144+
raise DuplicateRegistrationError(
145+
kind, wire_name, f"{owner}.{registry[wire_name]}", f"{owner}.{attr}",
146+
)
147+
registry[wire_name] = attr
148+
return handlers["signal"], handlers["query"], handlers["update"], handlers["update_validator"]
149+
150+
128151
def defn(*, name: str) -> Callable[[type[_WorkflowT]], type[_WorkflowT]]:
129152
"""Register a class as a workflow type under a language-neutral name.
130153
@@ -137,37 +160,7 @@ def defn(*, name: str) -> Callable[[type[_WorkflowT]], type[_WorkflowT]]:
137160
def wrap(cls: type[_WorkflowT]) -> type[_WorkflowT]:
138161
cls.__workflow_name__ = name # type: ignore[attr-defined]
139162
_validate_workflow_entrypoint(cls)
140-
signals: dict[str, str] = {}
141-
queries: dict[str, str] = {}
142-
updates: dict[str, str] = {}
143-
update_validators: dict[str, str] = {}
144-
145-
def register_handler(kind: str, handlers: dict[str, str], wire_name: str, attr: str) -> None:
146-
if wire_name in handlers:
147-
owner = f"{cls.__module__}.{cls.__qualname__}"
148-
raise DuplicateRegistrationError(
149-
kind, wire_name, f"{owner}.{handlers[wire_name]}", f"{owner}.{attr}",
150-
)
151-
handlers[wire_name] = attr
152-
153-
for attr in dir(cls):
154-
if attr.startswith("_"):
155-
continue
156-
member = getattr(cls, attr, None)
157-
signal_name = getattr(member, "__signal_name__", None)
158-
if isinstance(signal_name, str) and signal_name:
159-
if signal_name == MESSAGE_STREAM_SIGNAL:
160-
raise ValueError(f"signal name {signal_name!r} is reserved by the workflow runtime")
161-
register_handler("signal", signals, signal_name, attr)
162-
query_name = getattr(member, "__query_name__", None)
163-
if isinstance(query_name, str) and query_name:
164-
register_handler("query", queries, query_name, attr)
165-
update_name = getattr(member, "__update_name__", None)
166-
if isinstance(update_name, str) and update_name:
167-
register_handler("update", updates, update_name, attr)
168-
update_validator_name = getattr(member, "__update_validator_name__", None)
169-
if isinstance(update_validator_name, str) and update_validator_name:
170-
register_handler("update_validator", update_validators, update_validator_name, attr)
163+
signals, queries, updates, update_validators = _workflow_handler_registries(cls)
171164
cls.__workflow_signals__ = signals # type: ignore[attr-defined]
172165
cls.__workflow_queries__ = queries # type: ignore[attr-defined]
173166
cls.__workflow_updates__ = updates # type: ignore[attr-defined]
@@ -2588,6 +2581,7 @@ class Replayer:
25882581
def __init__(self, *, workflows: Iterable[type]) -> None:
25892582
self._workflows: dict[str, type] = {}
25902583
for workflow_cls in workflows:
2584+
_workflow_handler_registries(workflow_cls)
25912585
workflow_type = str(getattr(workflow_cls, "__workflow_name__", workflow_cls.__name__))
25922586
if workflow_type in self._workflows:
25932587
raise DuplicateRegistrationError(

‎tests/test_registration_ambiguity.py‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,34 @@ def replacement(self: Any) -> str:
114114
assert child().handler() == "replacement"
115115

116116

117+
@pytest.mark.parametrize("kind", ["signal", "query", "update", "update_validator"])
118+
@pytest.mark.parametrize("consumer", ["worker", "replayer", "environment"])
119+
def test_programmatic_workflow_admission_cannot_bypass_handler_validation(kind: str, consumer: str) -> None:
120+
decorate: Callable[..., Any] = getattr(workflow, kind)
121+
def first_handler(self: Any) -> None:
122+
pass
123+
124+
def second_handler(self: Any) -> None:
125+
pass
126+
127+
cls = type("ProgrammaticWorkflow", (), {
128+
"__workflow_name__": "programmatic-workflow", "run": run,
129+
"first_handler": decorate("approve")(first_handler),
130+
"second_handler": decorate("approve")(second_handler),
131+
})
132+
client = AsyncMock(spec=Client)
133+
with pytest.raises(DuplicateRegistrationError) as captured:
134+
if consumer == "worker":
135+
Worker(client, task_queue="registration", workflows=[cls])
136+
elif consumer == "replayer":
137+
workflow.Replayer(workflows=[cls])
138+
else:
139+
WorkflowEnvironment().register_workflow(cls)
140+
assert captured.value.registration_kind == kind
141+
assert captured.value.wire_name == "approve"
142+
assert client.mock_calls == []
143+
144+
117145
def test_handler_names_are_scoped_by_kind() -> None:
118146
cls = definition("scoped-handlers", "ScopedHandlers", {
119147
"signal": workflow.signal("approve")(lambda self: None),
@@ -157,7 +185,8 @@ def test_mock_replacement_is_an_explicit_test_setting() -> None:
157185

158186
def test_duplicate_error_is_a_public_structured_sdk_error() -> None:
159187
error = DuplicateRegistrationError("workflow", "duplicate", run, first)
160-
assert isinstance(error, (DurableWorkflowError, ValueError))
188+
assert isinstance(error, DurableWorkflowError)
189+
assert isinstance(error, ValueError)
161190
assert error.registration_kind == "workflow"
162191
assert error.wire_name == "duplicate"
163192
assert error.first_definition.endswith(".run")

0 commit comments

Comments
 (0)