From 06d73a5307de6f9ec3afc2c121cf27f6829292e1 Mon Sep 17 00:00:00 2001 From: Brad Stein Date: Wed, 21 Jan 2026 05:32:49 -0300 Subject: [PATCH] fix: simplify mailu sync retry --- ariadne/services/mailu.py | 88 +++++++++++++++++++++++++++++---------- tests/test_services.py | 17 ++++++-- 2 files changed, 81 insertions(+), 24 deletions(-) diff --git a/ariadne/services/mailu.py b/ariadne/services/mailu.py index 7cf7ca0..ef16e3a 100644 --- a/ariadne/services/mailu.py +++ b/ariadne/services/mailu.py @@ -39,11 +39,21 @@ class MailuUserSyncResult: updated: int = 0 skipped: int = 0 failures: int = 0 + mailboxes: int = 0 + + +@dataclass(frozen=True) +class MailuSyncContext: + username: str + user_id: str + mailu_email: str + app_password: str + updated: int + display_name: str class PasswordTooLongError(RuntimeError): pass - mailboxes: int = 0 @dataclass @@ -216,14 +226,17 @@ class MailuService: return True return self._is_service_account(user, username) - def _sync_user(self, conn: psycopg.Connection, user: dict[str, Any]) -> MailuUserSyncResult: + def _build_sync_context( + self, + user: dict[str, Any], + ) -> tuple[MailuSyncContext | None, MailuUserSyncResult | None]: username = self._username(user) if self._should_skip_user(user, username): - return MailuUserSyncResult(skipped=1) + return None, MailuUserSyncResult(skipped=1) user_id = self._user_id(user) if not user_id: - return MailuUserSyncResult(failures=1) + return None, MailuUserSyncResult(failures=1) attrs = user.get("attributes") if not isinstance(attrs, dict): @@ -237,46 +250,79 @@ class MailuService: enabled, updates, app_password = self._prepare_updates(username, attrs, mailu_email) if not enabled: - return MailuUserSyncResult(skipped=1) + return None, MailuUserSyncResult(skipped=1) if not self._apply_updates(user_id, updates, username): - return MailuUserSyncResult(failures=1) + return None, MailuUserSyncResult(failures=1) updated = 1 if updates else 0 + display_name = _display_name(user) + return ( + MailuSyncContext( + username=username, + user_id=user_id, + mailu_email=mailu_email, + app_password=app_password, + updated=updated, + display_name=display_name, + ), + None, + ) + + def _ensure_mailbox_with_retry( + self, + conn: psycopg.Connection, + ctx: MailuSyncContext, + ) -> tuple[bool, bool, bool]: mailbox_ok = False - failed = False rotated = False + failed = False try: - mailbox_ok = self._ensure_mailbox(conn, mailu_email, app_password, _display_name(user)) - except PasswordTooLongError as exc: + mailbox_ok = self._ensure_mailbox(conn, ctx.mailu_email, ctx.app_password, ctx.display_name) + except PasswordTooLongError: rotated = True app_password = random_password(24) try: - keycloak_admin.set_user_attribute(username, MAILU_APP_PASSWORD_ATTR, app_password) + keycloak_admin.set_user_attribute(ctx.username, MAILU_APP_PASSWORD_ATTR, app_password) logger.info( "mailu app password rotated", extra={ "event": "mailu_sync", "status": "updated", "detail": "app password exceeded bcrypt limit", - "username": username, + "username": ctx.username, }, ) - mailbox_ok = self._ensure_mailbox(conn, mailu_email, app_password, _display_name(user)) + mailbox_ok = self._ensure_mailbox(conn, ctx.mailu_email, app_password, ctx.display_name) except Exception as retry_exc: - self._log_sync_error(username, str(retry_exc)) + self._log_sync_error(ctx.username, str(retry_exc)) failed = True except Exception as exc: - self._log_sync_error(username, str(exc)) + self._log_sync_error(ctx.username, str(exc)) failed = True - result = MailuUserSyncResult(skipped=1, updated=updated) - if rotated: - result = MailuUserSyncResult(skipped=1, updated=max(updated, 1)) + return mailbox_ok, failed, rotated + + @staticmethod + def _build_sync_result( + updated: int, + mailbox_ok: bool, + failed: bool, + rotated: bool, + ) -> MailuUserSyncResult: if failed: - result = MailuUserSyncResult(failures=1, updated=updated) - elif mailbox_ok: - result = MailuUserSyncResult(processed=1, updated=updated, mailboxes=1) - return result + return MailuUserSyncResult(failures=1, updated=updated) + if mailbox_ok: + return MailuUserSyncResult(processed=1, updated=updated, mailboxes=1) + if rotated: + return MailuUserSyncResult(skipped=1, updated=max(updated, 1)) + return MailuUserSyncResult(skipped=1, updated=updated) + + def _sync_user(self, conn: psycopg.Connection, user: dict[str, Any]) -> MailuUserSyncResult: + ctx, early = self._build_sync_context(user) + if early is not None: + return early + mailbox_ok, failed, rotated = self._ensure_mailbox_with_retry(conn, ctx) + return self._build_sync_result(ctx.updated, mailbox_ok, failed, rotated) def _ensure_mailbox( self, diff --git a/tests/test_services.py b/tests/test_services.py index ec4889c..3ce2574 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -466,9 +466,16 @@ def test_mailu_sync_retries_on_password_limit(monkeypatch) -> None: mailu_system_password="", ) monkeypatch.setattr("ariadne.services.mailu.settings", dummy_settings) - monkeypatch.setattr("ariadne.services.mailu.keycloak_admin.ready", lambda: True) + monkeypatch.setattr(mailu_module.keycloak_admin, "ready", lambda: True) + update_calls: list[tuple[str, dict[str, object]]] = [] monkeypatch.setattr( - "ariadne.services.mailu.keycloak_admin.iter_users", + mailu_module.keycloak_admin, + "update_user_safe", + lambda user_id, payload: update_calls.append((user_id, payload)), + ) + monkeypatch.setattr( + mailu_module.keycloak_admin, + "iter_users", lambda *args, **kwargs: [ { "id": "1", @@ -485,7 +492,8 @@ def test_mailu_sync_retries_on_password_limit(monkeypatch) -> None: set_calls: list[tuple[str, str, str]] = [] monkeypatch.setattr( - "ariadne.services.mailu.keycloak_admin.set_user_attribute", + mailu_module.keycloak_admin, + "set_user_attribute", lambda username, key, value: set_calls.append((username, key, value)), ) @@ -513,8 +521,11 @@ def test_mailu_sync_retries_on_password_limit(monkeypatch) -> None: summary = svc.sync("provision", force=True) assert summary.processed == 1 + assert update_calls assert call_count["count"] == 2 assert set_calls + + def test_mailu_sync_skips_disabled(monkeypatch) -> None: dummy_settings = types.SimpleNamespace( mailu_domain="bstein.dev",