-
Notifications
You must be signed in to change notification settings - Fork 16
feat(server): add outbound webhook dispatcher for note sync events #217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
proscar87
wants to merge
3
commits into
allenporter:main
Choose a base branch
from
proscar87:feat/event-webhook-dispatcher
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,4 +12,5 @@ | |
| "summary", | ||
| "system", | ||
| "user", | ||
| "webhook", | ||
| ] | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| """Data models for outbound webhook payloads. | ||
|
|
||
| See :mod:`supernote.server.services.webhook` for the dispatcher that builds | ||
| and signs these payloads before sending them to configured endpoints. | ||
| """ | ||
|
|
||
| from dataclasses import dataclass | ||
|
|
||
| from mashumaro.mixins.json import DataClassJSONMixin | ||
|
|
||
|
|
||
| @dataclass | ||
| class WebhookFileVO(DataClassJSONMixin): | ||
| """File metadata included in a `note.sync_completed` webhook payload.""" | ||
|
|
||
| id: int | ||
| """The internal ID of the synced file.""" | ||
|
|
||
| path: str | ||
| """The full path of the file, relative to the user's root directory.""" | ||
|
|
||
|
|
||
| @dataclass | ||
| class NoteSyncCompletedPayload(DataClassJSONMixin): | ||
| """Payload sent for the `note.sync_completed` outbound webhook event. | ||
|
|
||
| Corresponds to `EVENT_NOTE_SYNC_COMPLETED` in | ||
| `supernote.server.services.webhook`. | ||
|
|
||
| Intentionally excludes the internal `user_id` (a private, per-user | ||
| database identifier) -- see that module for discussion. | ||
| """ | ||
|
|
||
| event: str | ||
| """The webhook event name. Always `note.sync_completed` today.""" | ||
|
|
||
| timestamp: int | ||
| """Unix timestamp (seconds) of when the event was dispatched.""" | ||
|
|
||
| file: WebhookFileVO | ||
| """Metadata about the file that finished syncing.""" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| """Outbound webhook dispatch for server events. | ||
|
|
||
| Listens on the internal :class:`~supernote.server.events.LocalEventBus` and, | ||
| for every configured endpoint subscribed to a given event name, sends a | ||
| signed HTTP POST notification. Delivery is best-effort: a slow or failing | ||
| endpoint is logged and skipped, it never raises back into the caller and | ||
| never blocks or delays the operation that triggered the event (e.g. a | ||
| device sync request). | ||
| """ | ||
|
|
||
| import asyncio | ||
| import hashlib | ||
| import hmac | ||
| import json | ||
| import logging | ||
| import time | ||
| from typing import Any | ||
|
|
||
| import aiohttp | ||
|
|
||
| from supernote.models.webhook import NoteSyncCompletedPayload, WebhookFileVO | ||
|
|
||
| from ..config import WebhookConfig, WebhookEndpointConfig | ||
| from ..events import Event, LocalEventBus, NoteUpdatedEvent | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| # Name of the event dispatched when a .note file finishes syncing to the | ||
| # server. Fired from the same point the processing pipeline is enqueued | ||
| # (see NoteUpdatedEvent), i.e. as soon as the device's upload completes. | ||
| EVENT_NOTE_SYNC_COMPLETED = "note.sync_completed" | ||
|
|
||
| SIGNATURE_HEADER = "X-Supernote-Signature" | ||
| EVENT_HEADER = "X-Supernote-Event" | ||
|
|
||
| REQUEST_TIMEOUT_SECONDS = 10.0 | ||
| MAX_ATTEMPTS = 3 | ||
| RETRY_BACKOFF_SECONDS = 1.0 | ||
|
|
||
|
|
||
| def sign_payload(secret: str, body: bytes) -> str: | ||
| """Compute the `sha256=<hex>` HMAC signature for a webhook payload body.""" | ||
| digest = hmac.new(secret.encode("utf-8"), body, hashlib.sha256).hexdigest() | ||
| return f"sha256={digest}" | ||
|
|
||
|
|
||
| class WebhookService: | ||
| """Dispatches signed outbound webhook notifications for subscribed events.""" | ||
|
|
||
| def __init__(self, config: WebhookConfig, event_bus: LocalEventBus) -> None: | ||
| self.config = config | ||
| self.event_bus = event_bus | ||
|
|
||
| def start(self) -> None: | ||
| """Subscribe to the events that can trigger webhook dispatch.""" | ||
| if not self.config.enabled or not self.config.endpoints: | ||
| logger.debug("Webhook dispatch disabled or no endpoints configured.") | ||
| return | ||
| self.event_bus.subscribe(NoteUpdatedEvent, self._handle_note_updated) | ||
| logger.info( | ||
| f"WebhookService started with {len(self.config.endpoints)} endpoint(s)." | ||
| ) | ||
|
|
||
| async def _handle_note_updated(self, event: Event) -> None: | ||
| """Translate a NoteUpdatedEvent into a note.sync_completed webhook.""" | ||
| if not isinstance(event, NoteUpdatedEvent): | ||
| return | ||
| # Note: intentionally does not include event.user_id. It's an | ||
| # internal database identifier, not something we want to hand to | ||
| # third-party endpoints. See PR #217 for discussion on how | ||
| # per-user webhook delivery might work in a future iteration. | ||
| payload = NoteSyncCompletedPayload( | ||
| event=EVENT_NOTE_SYNC_COMPLETED, | ||
| timestamp=int(time.time()), | ||
| file=WebhookFileVO(id=event.file_id, path=event.file_path), | ||
| ) | ||
| await self.dispatch(EVENT_NOTE_SYNC_COMPLETED, payload.to_dict()) | ||
|
|
||
| async def dispatch(self, event_name: str, payload: dict[str, Any]) -> None: | ||
| """Send `payload` to every configured endpoint subscribed to `event_name`. | ||
|
|
||
| An endpoint with no `events` configured is subscribed to everything | ||
| (see `WebhookEndpointConfig.events`). Never raises: endpoints are | ||
| notified concurrently and independently, and one endpoint failing has | ||
| no effect on delivery to the others. | ||
| """ | ||
| endpoints = [ | ||
| ep | ||
| for ep in self.config.endpoints | ||
| if not ep.events or event_name in ep.events | ||
| ] | ||
| if not endpoints: | ||
| return | ||
| results = await asyncio.gather( | ||
| *(self._send(endpoint, event_name, payload) for endpoint in endpoints), | ||
| return_exceptions=True, | ||
| ) | ||
| for endpoint, result in zip(endpoints, results, strict=True): | ||
| if isinstance(result, BaseException): | ||
| logger.error( | ||
| f"Webhook {event_name} to {endpoint.url} raised " | ||
| f"{type(result).__name__}: {result}" | ||
| ) | ||
|
|
||
| async def _send( | ||
| self, | ||
| endpoint: WebhookEndpointConfig, | ||
| event_name: str, | ||
| payload: dict[str, Any], | ||
| ) -> None: | ||
| """POST `payload` to a single endpoint, retrying transient failures. | ||
|
|
||
| Client errors (4xx) are not retried. Network errors, timeouts, and | ||
| server errors (5xx) are retried up to MAX_ATTEMPTS with a linear | ||
| backoff. All failures are logged (without the secret or signature) | ||
| and swallowed. | ||
| """ | ||
| body = json.dumps(payload).encode("utf-8") | ||
| headers = {"Content-Type": "application/json", EVENT_HEADER: event_name} | ||
| if endpoint.secret: | ||
| headers[SIGNATURE_HEADER] = sign_payload(endpoint.secret, body) | ||
|
|
||
| timeout = aiohttp.ClientTimeout(total=REQUEST_TIMEOUT_SECONDS) | ||
| for attempt in range(1, MAX_ATTEMPTS + 1): | ||
| try: | ||
| async with aiohttp.ClientSession(timeout=timeout) as session: | ||
| async with session.post( | ||
| endpoint.url, data=body, headers=headers | ||
| ) as response: | ||
| if response.status < 400: | ||
| return | ||
| if response.status < 500: | ||
| logger.warning( | ||
| f"Webhook {event_name} to {endpoint.url} rejected " | ||
| f"with status {response.status}; not retrying." | ||
| ) | ||
| return | ||
| logger.warning( | ||
| f"Webhook {event_name} to {endpoint.url} failed with " | ||
| f"status {response.status} (attempt {attempt}/{MAX_ATTEMPTS})." | ||
| ) | ||
| except (aiohttp.ClientError, TimeoutError) as e: | ||
| logger.warning( | ||
| f"Webhook {event_name} to {endpoint.url} failed: " | ||
| f"{type(e).__name__} (attempt {attempt}/{MAX_ATTEMPTS})." | ||
| ) | ||
|
|
||
| if attempt < MAX_ATTEMPTS: | ||
| await asyncio.sleep(RETRY_BACKOFF_SECONDS * attempt) | ||
|
|
||
| logger.error( | ||
| f"Webhook {event_name} to {endpoint.url} failed after " | ||
| f"{MAX_ATTEMPTS} attempts; giving up." | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| """Tests for outbound webhook payload models.""" | ||
|
|
||
| from supernote.models.webhook import NoteSyncCompletedPayload, WebhookFileVO | ||
|
|
||
|
|
||
| def test_note_sync_completed_payload_serializes_to_dict() -> None: | ||
| payload = NoteSyncCompletedPayload( | ||
| event="note.sync_completed", | ||
| timestamp=1730000000, | ||
| file=WebhookFileVO(id=42, path="Note/Daily/2026-08-04.note"), | ||
| ) | ||
|
|
||
| assert payload.to_dict() == { | ||
| "event": "note.sync_completed", | ||
| "timestamp": 1730000000, | ||
| "file": {"id": 42, "path": "Note/Daily/2026-08-04.note"}, | ||
| } | ||
|
|
||
|
|
||
| def test_note_sync_completed_payload_excludes_user_id() -> None: | ||
| """The payload has no field for the internal, per-user database ID.""" | ||
| payload = NoteSyncCompletedPayload( | ||
| event="note.sync_completed", | ||
| timestamp=1730000000, | ||
| file=WebhookFileVO(id=1, path="a.note"), | ||
| ) | ||
|
|
||
| assert "user_id" not in payload.to_dict() | ||
| assert not hasattr(payload, "user_id") |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you expect this to provide security in practice?