✨(jmap) add JMAP endpoint by sylvinus · Pull Request #479 · suitenumerique/messages

sylvinus · 2026-01-13T20:17:32Z

This is the first step in our standards-compliant JMAP support.

For now it implements just enough to be able to list recent threads and send emails.

We should:

Implement a bit more of the spec
Test it with actual JMAP clients (& check OIDC login)
Make this endpoint enabled with FEATURE_JMAP or something similar
Request feedback from the JMAP community

Summary by CodeRabbit

New Features
- Added complete JMAP API support enabling efficient queries of mailboxes, emails, and threads
- Implemented JMAP session management endpoint for full protocol compatibility
- Added comprehensive error handling with standardized JMAP response formatting
Tests
- Added extensive test coverage for JMAP methods, session management, and end-to-end mail workflows

_{✏️ Tip: You can customize this high-level summary in your review settings.}

This is the first step in our standards-compliant JMAP support. It implements just enough to be able to list recent threads.

coderabbitai · 2026-01-13T20:17:44Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

🔍 Trigger a full review

📝 Walkthrough

Walkthrough

This PR introduces a complete JMAP (RFC 8620) API implementation for the backend, including custom exception definitions, method handlers with back-reference resolution and property filtering, Django REST views for session and method-call processing, URL routing integration, and comprehensive test infrastructure.

Changes

Cohort / File(s)	Summary
JMAP Module Foundation `src/backend/core/api/jmap/__init__.py`	Module docstring defining JMAP API package
Error Handling `src/backend/core/api/jmap/errors.py`	Seven custom exception classes (base JMAPError plus subclasses: UnknownCapabilityError, UnknownMethodError, InvalidArgumentsError, InvalidResultReferenceError, AccountNotFoundError, ForbiddenError, ServerFailError) with standard JMAP error response formatting
Method Registry & Handlers `src/backend/core/api/jmap/methods.py`	MethodRegistry for handler dispatch; JMAPContext for execution state; BaseMethod abstract base with back-reference resolution and utility helpers; five public JMAP method implementations (MailboxQuery, MailboxGet, EmailQuery, EmailGet, ThreadGet) with filtering, pagination, property selection, and serialization
REST Endpoints `src/backend/core/api/jmap/views.py`	JMAPSessionView (GET) exposing session capabilities, accounts, and URLs; JMAPAPIView (POST) processing method calls with argument resolution, error handling, and result accumulation
URL Configuration `src/backend/core/urls.py`	Two new JMAP endpoints: `/api/{version}/jmap/session` and `/api/{version}/jmap/`
Test Infrastructure `src/backend/core/tests/api/jmap/__init__.py`, `src/backend/core/tests/api/jmap/conftest.py`	Test package initialization; pytest fixtures (api_client, user, mailbox, mailbox_with_threads, jmap_client); JMAPTestClient for method invocation with reference handling and serialization; JMAPTestResponse and JMAPTestError utilities
Test Modules `src/backend/core/tests/api/jmap/test_jmap_session.py`, `src/backend/core/tests/api/jmap/test_jmap_methods.py`, `src/backend/core/tests/api/jmap/test_jmap_workflow.py`	Session endpoint tests (capabilities, accounts, authentication); method-specific tests (MailboxQuery/Get, EmailQuery/Get, ThreadGet with filtering, sorting, properties); end-to-end workflow tests (multi-thread scenarios, date filtering, multiple mailboxes)
Dependencies `src/backend/pyproject.toml`	Added jmapc==0.2.23 as optional dev dependency

Sequence Diagram

sequenceDiagram
    participant Client
    participant JMAPAPIView
    participant MethodRegistry
    participant BaseMethod
    participant Database

    Client->>JMAPAPIView: POST /jmap/ (methodCalls)
    JMAPAPIView->>JMAPAPIView: Validate request structure & capabilities
    
    loop For each methodCall
        JMAPAPIView->>JMAPAPIView: resolve_args (back-references)
        JMAPAPIView->>MethodRegistry: get_handler(method_name)
        MethodRegistry-->>JMAPAPIView: Handler class
        JMAPAPIView->>BaseMethod: execute(args, context)
        
        alt Account validation
            BaseMethod->>BaseMethod: _get_account_id(accountId)
        end
        
        alt Back-reference resolution
            BaseMethod->>BaseMethod: resolve_value(value, context)
            BaseMethod->>BaseMethod: _resolve_reference(ref)
            BaseMethod->>BaseMethod: _navigate_path(obj, path)
        end
        
        BaseMethod->>Database: Query (filter, sort, paginate)
        Database-->>BaseMethod: Results
        BaseMethod->>BaseMethod: Serialize results (mailbox/email)
        BaseMethod-->>JMAPAPIView: Method response
        JMAPAPIView->>JMAPAPIView: Accumulate in results[callId]
    end
    
    JMAPAPIView-->>Client: Response (methodResponses, sessionState)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

The changes introduce substantial new functionality with high logic density: multiple JMAP method implementations with overlapping but distinct business logic (filtering, sorting, pagination, property selection), intricate back-reference resolution with nested path traversal, and comprehensive error handling. The heterogeneous nature of individual method handlers—each with unique database queries and serialization—combined with the scope of new public APIs and supporting infrastructure (views, registry, test client) requires careful, multi-faceted review.

Suggested reviewers

sdemagny
Nastaliss

Poem

🐰 JMAP now hops through backend halls,
Methods dance and back-refs call,
Sessions bloom with capability's grace,
Threads and emails find their place!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: adding a JMAP endpoint. It directly corresponds to the primary deliverable of this PR, which introduces JMAP session and API views with supporting infrastructure.
Docstring Coverage	✅ Passed	Docstring coverage is 82.09% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch jmap

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

src/backend/core/api/jmap/views.py

+            "sessionState": datetime.now(dt_timezone.utc).isoformat(),
+        }
+
+        return Response(response)


In general, the fix is to avoid returning raw exception messages in responses. Instead, log the full exception (including stack trace) on the server side and send back a generic error description to the client, following the JMAP spec’s serverFail semantics or a similar pattern.

Concretely, in JMAPAPIView.post, update the generic except Exception as e: handler to:

Stop including str(e) in the description field.

Replace it with a generic message such as "An internal server error occurred." or omit description altogether if that’s acceptable for your client.

Optionally, log the exception server-side (e.g., via Python’s logging module) so developers can still see the details while users cannot.

To implement this with minimal functional change:

Add an import for Python’s logging module at the top of src/backend/core/api/jmap/views.py.

Create a module-level logger (e.g., logger = logging.getLogger(__name__)).

In the except Exception as e: block, call logger.exception("Unhandled exception while processing JMAP method %s", method_name) to log the full stack trace.

Replace {"type": "serverFail", "description": str(e)} with {"type": "serverFail", "description": "An internal server error occurred."} (or similarly generic text).

All changes are confined to src/backend/core/api/jmap/views.py within the shown snippets: adding the logging import and logger definition near the top, and modifying the except Exception block in JMAPAPIView.post.

coderabbitai

Actionable comments posted: 4

🤖 Fix all issues with AI agents

In @src/backend/core/api/jmap/methods.py:
- Around line 179-195: The returned "total" is computed after pagination;
compute total_count = len(mailbox_ids) immediately after mailbox_ids =
list(mailboxes.values_list("id", flat=True)) and before applying the
position/limit slice, then use total_count in the returned dict's "total" field
while still slicing mailbox_ids for "ids"; adjust references in the method
(mailbox_ids, position, limit, and the returned dict) so "ids" contains the
paginated IDs but "total" reflects the full unpaged match count.
- Around line 105-119: Refactor the loop in _navigate_path to enumerate parts
(for idx, part in enumerate(parts)) so when part == "*" you compute
remaining_parts = parts[idx + 1:] (not parts.index(part)) to handle multiple
wildcards correctly; also restructure the control flow so there is no elif after
a return (e.g., use an if/return and then continue or fallthrough for other
cases) to satisfy the no-else-return lint rule, and build remaining_path = "/" +
"/".join(remaining_parts) and return [self._navigate_path(item, remaining_path)
for item in current] or current as before.

In @src/backend/core/api/jmap/views.py:
- Around line 181-189: The current except block in views.py appends str(e) into
method_responses which can leak internal details; change the handler in the
except Exception block (where method_responses.append([...]) is called) to stop
returning str(e) to the client—replace the response description with a generic
message like "Internal server error" or "serverFail" and record the real
exception internally by calling capture_exception() (from sentry_sdk) or logging
it via your logger before appending the generic error response; ensure call_id
and error type remain unchanged so clients can correlate errors without seeing
internal details.

In @src/backend/core/tests/api/jmap/test_jmap_methods.py:
- Around line 31-54: The test contains a local import "import uuid" inside
test_mailbox_query_filters_by_name; move that import to the module top-level
(add "import uuid" to the file imports) and remove the inline import from the
test function so test_mailbox_query_filters_by_name uses the top-level uuid
import instead of importing inside the function.

🧹 Nitpick comments (8)

src/backend/core/urls.py (1)

228-238: Consider adding trailing slash to session endpoint for consistency.

The /jmap/session endpoint lacks a trailing slash while /jmap/ has one. While this might be intentional per JMAP spec conventions, Django typically expects trailing slashes. Consider either:

Adding trailing slash: jmap/session/

Ensuring APPEND_SLASH setting handles this appropriately

This is a minor consistency observation - if the current pattern matches JMAP client expectations, it's fine to keep as-is.
src/backend/core/tests/api/jmap/test_jmap_session.py (1)
26-51: Address unused mailbox argument warnings.

The mailbox fixture is required to create the mailbox in the database (side effect), but Pylint warns about unused arguments. Use underscore prefix or explicit annotation to indicate intentional side-effect usage.
♻️ Suggested fix
-    def test_session_returns_account(self, api_client, user, mailbox):
+    def test_session_returns_account(self, api_client, user, mailbox):  # noqa: ARG002
         """Test that the session includes the user's account."""
Or rename to underscore prefix:
-    def test_session_returns_account(self, api_client, user, mailbox):
+    def test_session_returns_account(self, api_client, user, _mailbox):
         """Test that the session includes the user's account."""
Apply the same pattern to test_session_returns_primary_account at line 40.
src/backend/core/api/jmap/views.py (1)
61-68: Simplify first mailbox retrieval.

The for-loop pattern to get the first item is unnecessarily verbose.
♻️ Proposed simplification
         # Use the first mailbox email as the account name, or user email
-        primary_email = None
-        for mailbox in mailboxes[:1]:
-            primary_email = f"{mailbox.local_part}@{mailbox.domain.name}"
-            break
+        first_mailbox = mailboxes.first()
+        primary_email = (
+            f"{first_mailbox.local_part}@{first_mailbox.domain.name}"
+            if first_mailbox
+            else None
+        )
src/backend/core/tests/api/jmap/test_jmap_workflow.py (1)
144-150: Move import to top of file.

The Message import inside the test function triggers linting warnings. Move it to the top-level imports for consistency and linter compliance.
♻️ Proposed fix

At the top of the file (around line 21):
 from core import enums, factories
+from core.models import Message
Then remove line 145:
-        from core.models import Message
-
         Message.objects.filter(id=recent_msg.id).update(
src/backend/core/tests/api/jmap/conftest.py (1)
317-321: Minor: Remove unnecessary pass statement.

Exception classes with docstrings don't need an explicit pass.
♻️ Proposed fix
 class JMAPTestError(Exception):
     """Exception raised when a JMAP method call fails."""
-
-    pass
src/backend/core/api/jmap/methods.py (3)
242-253: N+1 query problem: 4 COUNT queries per mailbox.

Each call to _serialize_mailbox executes 4 separate database queries for counts. For M mailboxes, this results in 4M additional queries, which will degrade performance significantly with many mailboxes.

Consider annotating the queryset with counts using Django's Count and Case/When:
from django.db.models import Count, Case, When, IntegerField

mailboxes = models.Mailbox.objects.filter(
    accesses__user=self.context.user
).annotate(
    total_emails=Count('accesses__thread__messages', distinct=True),
    unread_emails=Count(
        Case(When(accesses__thread__messages__is_unread=True, then=1)),
        output_field=IntegerField()
    ),
    # ... similar for threads
)
This would reduce the query count from O(4M) to O(1).

406-411: N+1 query: ThreadAccess query inside serialization loop.

Each email serialization executes a separate query for mailbox_ids. With N emails, this adds N extra queries.
Consider prefetching or batching

You could prefetch thread__accesses in the main query:
         messages = (
             models.Message.objects.filter(id__in=ids)
             .filter(thread__accesses__mailbox__accesses__user=self.context.user)
-            .select_related("sender", "thread", "blob")
-            .prefetch_related("recipients__contact")
+            .select_related("sender", "thread", "blob")
+            .prefetch_related("recipients__contact", "thread__accesses")
         )
Then in _serialize_email:
mailbox_ids = [access.mailbox_id for access in message.thread.accesses.all()]
481-491: Potential prefetch inefficiency: .order_by() may bypass prefetch cache.

Calling .order_by("created_at") on thread.messages inside the loop may issue a new database query per thread, bypassing the prefetch_related("messages") optimization.
Use Prefetch with ordering or sort in Python

Option 1 - Use Prefetch with ordering:
from django.db.models import Prefetch

threads = models.Thread.objects.filter(
    id__in=ids, accesses__mailbox__accesses__user=self.context.user
).prefetch_related(
    Prefetch("messages", queryset=models.Message.objects.order_by("created_at"))
)
Then remove the .order_by():
"emailIds": [str(m.id) for m in thread.messages.all()],
Option 2 - Sort in Python:
"emailIds": [str(m.id) for m in sorted(thread.messages.all(), key=lambda m: m.created_at)],

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bef485a and 3c941dc.

⛔ Files ignored due to path filters (1)

src/backend/poetry.lock is excluded by !**/*.lock

📒 Files selected for processing (11)

src/backend/core/api/jmap/__init__.py
src/backend/core/api/jmap/errors.py
src/backend/core/api/jmap/methods.py
src/backend/core/api/jmap/views.py
src/backend/core/tests/api/jmap/__init__.py
src/backend/core/tests/api/jmap/conftest.py
src/backend/core/tests/api/jmap/test_jmap_methods.py
src/backend/core/tests/api/jmap/test_jmap_session.py
src/backend/core/tests/api/jmap/test_jmap_workflow.py
src/backend/core/urls.py
src/backend/pyproject.toml

🧰 Additional context used

📓 Path-based instructions (6)

src/backend/**/*.py