diff --git a/README.md b/README.md index 831b7b0..16547dd 100644 --- a/README.md +++ b/README.md @@ -169,8 +169,13 @@ metadata. Every applied transition has a Poll-owned lifecycle audit record. Re-deciding always appends a record and supersedes the current decision. An exact retry carrying the same `Idempotency-Key` is a no-op and returns the original transition record; reusing that key for a different action or option -is rejected. Without an idempotency identity, a repeated transition is invalid -except for the deliberately auditable `decided` → `decided` action. +is rejected. A command whose requested lifecycle state already holds is also +an idempotent domain no-op, even without an idempotency identity. It returns +the current Poll with `replayed=true` and `transition=null`, does not append a +lifecycle audit record, and is emitted only to operational logs as duplicate +command telemetry. A new idempotency key supplied for such an audit-free no-op +is not consumed. Re-deciding with a different option remains a new auditable +action; repeating the current decision is a no-op. Poll API representations expose every lifecycle action with its availability and, when unavailable, the policy reason. Management clients can use the diff --git a/src/govoplan_poll/backend/router.py b/src/govoplan_poll/backend/router.py index a8f12a5..e827e75 100644 --- a/src/govoplan_poll/backend/router.py +++ b/src/govoplan_poll/backend/router.py @@ -100,8 +100,12 @@ def _poll_response(poll) -> PollResponse: def _transition_status_response(result) -> PollStatusResponse: return PollStatusResponse( poll=_poll_response(result.poll), - transition=PollLifecycleTransitionResponse.model_validate( - poll_lifecycle_transition_response(result.transition) + transition=( + PollLifecycleTransitionResponse.model_validate( + poll_lifecycle_transition_response(result.transition) + ) + if result.transition is not None + else None ), replayed=result.replayed, ) diff --git a/src/govoplan_poll/backend/service.py b/src/govoplan_poll/backend/service.py index e30e830..8e2bc5e 100644 --- a/src/govoplan_poll/backend/service.py +++ b/src/govoplan_poll/backend/service.py @@ -1,6 +1,7 @@ from __future__ import annotations import hashlib +import logging import re import secrets from dataclasses import dataclass @@ -24,6 +25,9 @@ from govoplan_poll.backend.schemas import ( from govoplan_poll.backend.transitions import DEFAULT_POLL_TRANSITION_ENGINE, PollTransitionEngine +logger = logging.getLogger("govoplan.poll.lifecycle") + + class PollError(ValueError): pass @@ -67,7 +71,7 @@ YES_NO_MAYBE_OPTIONS = ( @dataclass(frozen=True) class PollTransitionResult: poll: Poll - transition: PollLifecycleTransition + transition: PollLifecycleTransition | None replayed: bool = False @@ -721,6 +725,77 @@ def _replayed_transition( return PollTransitionResult(poll=poll, transition=existing, replayed=True) +def _latest_lifecycle_transition( + session: Session, + *, + poll_id: str, +) -> PollLifecycleTransition | None: + return ( + session.query(PollLifecycleTransition) + .filter(PollLifecycleTransition.poll_id == poll_id) + .order_by( + PollLifecycleTransition.created_at.desc(), + PollLifecycleTransition.id.desc(), + ) + .first() + ) + + +def _is_exact_transition_noop( + session: Session, + *, + poll: Poll, + action: str, + decision_option: PollOption | None, + transition_engine: PollTransitionEngine, +) -> bool: + """Recognize commands whose requested lifecycle state already holds.""" + + rule = transition_engine.policy.rules.get(action) + if rule is None: + return False + if action == "decide": + return ( + rule.target_status == poll.status + and decision_option is not None + and decision_option.id == poll.decided_option_id + ) + if rule.restore_archived_status: + latest = _latest_lifecycle_transition(session, poll_id=poll.id) + return ( + latest is not None + and latest.action == action + and latest.to_status == poll.status + ) + return rule.target_status == poll.status + + +def _log_duplicate_transition( + *, + poll: Poll, + action: str, + actor_user_id: str | None, + actor_api_key_id: str | None, + reason: str, + has_idempotency_key: bool, +) -> None: + """Record command duplication as operational telemetry, not lifecycle audit.""" + + logger.info( + "Ignored repeated exact Poll lifecycle transition", + extra={ + "event": "poll.lifecycle_transition.duplicate", + "tenant_id": poll.tenant_id, + "poll_id": poll.id, + "transition_action": action, + "actor_user_id": actor_user_id, + "actor_api_key_id": actor_api_key_id, + "duplicate_reason": reason, + "has_idempotency_key": has_idempotency_key, + }, + ) + + def _clear_expired_close_for_open(poll: Poll, *, action: str, now: datetime) -> datetime | None: if action != "open" or poll.closes_at is None: return None @@ -793,7 +868,7 @@ def transition_poll( mutation_owner: PollMutationOwner | None = None, transition_engine: PollTransitionEngine = DEFAULT_POLL_TRANSITION_ENGINE, ) -> PollTransitionResult: - """Apply one policy-controlled transition and append its durable audit record.""" + """Apply a policy transition, or return an audit-free exact no-op.""" poll = _lock_poll_for_transition(session, tenant_id=tenant_id, poll_id=poll_id) _assert_poll_mutation_owner(poll, mutation_owner=mutation_owner) @@ -817,8 +892,33 @@ def transition_poll( decision_option=decision_option, ) if replayed is not None: + _log_duplicate_transition( + poll=poll, + action=action, + actor_user_id=actor_user_id, + actor_api_key_id=actor_api_key_id, + reason="idempotency_key_replay", + has_idempotency_key=True, + ) return replayed + if _is_exact_transition_noop( + session, + poll=poll, + action=action, + decision_option=decision_option, + transition_engine=transition_engine, + ): + _log_duplicate_transition( + poll=poll, + action=action, + actor_user_id=actor_user_id, + actor_api_key_id=actor_api_key_id, + reason="requested_state_already_active", + has_idempotency_key=normalized_key is not None, + ) + return PollTransitionResult(poll=poll, transition=None, replayed=True) + try: plan = transition_engine.plan( current_status=poll.status, diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index 5149d1b..fccc7a6 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -266,12 +266,86 @@ class PollLifecycleTests(unittest.TestCase): idempotency_key="decision-2", ) - def test_repeated_exact_transition_without_an_identity_is_rejected(self) -> None: + def test_repeated_exact_transition_without_an_identity_is_an_audit_free_noop(self) -> None: + poll = self._poll() + applied = transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="open") + + with self.assertLogs("govoplan.poll.lifecycle", level="INFO") as logs: + repeated = transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + action="open", + actor_user_id="owner", + ) + + self.assertFalse(applied.replayed) + self.assertTrue(repeated.replayed) + self.assertIsNone(repeated.transition) + self.assertEqual(poll.status, "open") + self.assertEqual(len(poll.lifecycle_transitions), 1) + self.assertIn("Ignored repeated exact Poll lifecycle transition", logs.output[0]) + + def test_same_decision_is_a_noop_but_a_changed_decision_is_audited(self) -> None: poll = self._poll() transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="open") + transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="close") + first = transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + action="decide", + option_key="yes", + ) + repeated = transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + action="decide", + option_key="yes", + ) + changed = transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + action="decide", + option_key="no", + ) + self.assertTrue(repeated.replayed) + self.assertIsNone(repeated.transition) + self.assertIsNotNone(first.transition) + self.assertIsNotNone(changed.transition) + self.assertEqual(changed.transition.previous_decision_option_id, first.transition.decision_option_id) + self.assertEqual( + [item.action for item in poll.lifecycle_transitions], + ["open", "close", "decide", "decide"], + ) + + def test_exact_unarchive_retry_is_a_noop_but_unarchive_without_history_is_invalid(self) -> None: + poll = self._poll() + transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="archive") + transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="unarchive") + + repeated = transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + action="unarchive", + ) + + self.assertTrue(repeated.replayed) + self.assertIsNone(repeated.transition) + self.assertEqual([item.action for item in poll.lifecycle_transitions], ["archive", "unarchive"]) + + never_archived = self._poll(title="Never archived") with self.assertRaisesRegex(PollError, "not allowed"): - transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="open") + transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=never_archived.id, + action="unarchive", + ) def test_archive_and_unarchive_restore_status_and_append_audit_history(self) -> None: poll = self._poll() @@ -444,6 +518,7 @@ class PollLifecycleTests(unittest.TestCase): self.assertEqual(opened.poll.status, "open") self.assertFalse(opened.replayed) self.assertTrue(replay.replayed) + self.assertIsNotNone(replay.transition) self.assertEqual(len(lifecycle.history), 1) self.assertEqual(lifecycle.history[0].actor_user_id, "owner-membership") self.assertEqual( @@ -462,6 +537,27 @@ class PollLifecycleTests(unittest.TestCase): ) self.assertEqual(rejected.exception.status_code, 400) + def test_generic_api_returns_null_transition_for_domain_noop(self) -> None: + poll = self._poll(status="open", title="Already open") + + repeated = api_transition_poll( + poll.id, + PollTransitionRequest(action="open"), + idempotency_key=None, + session=self.session, + principal=self._principal(), + ) + + self.assertEqual(repeated.poll.status, "open") + self.assertTrue(repeated.replayed) + self.assertIsNone(repeated.transition) + self.assertEqual( + self.session.query(PollLifecycleTransition) + .filter(PollLifecycleTransition.poll_id == poll.id) + .count(), + 0, + ) + if __name__ == "__main__": unittest.main()