From e581b947a759b58e8ebb69496588bceb2ed36318 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed Date: Tue, 31 Mar 2026 11:37:54 +0500 Subject: [PATCH 1/2] docs: Add ADR for ensuring GET requests are idempotent Add edx-platform/docs/decisions/0030-ensure-get-requests-are-idempotent.rst as an accepted ADR. Define policy that GET endpoints must be strictly read-only, with side effects moved to explicit write endpoints or async event pipelines. Include edx-platform relevance, anti-pattern vs preferred code examples, and rollout guidance for testing and migration. --- ...030-ensure-get-requests-are-idempotent.rst | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 docs/decisions/0030-ensure-get-requests-are-idempotent.rst diff --git a/docs/decisions/0030-ensure-get-requests-are-idempotent.rst b/docs/decisions/0030-ensure-get-requests-are-idempotent.rst new file mode 100644 index 000000000000..8bf1830eaa3d --- /dev/null +++ b/docs/decisions/0030-ensure-get-requests-are-idempotent.rst @@ -0,0 +1,91 @@ +Ensure GET is Idempotent +=============================== + +:Status: Proposed +:Date: 2026-03-31 +:Deciders: API Working Group + +Context +======= + +Some Open edX endpoints use ``GET`` requests that have side-effects (e.g., writing tracking logs, +recording first access events). This violates REST safety/idempotency expectations and can break +caching/proxy behavior and automated clients/agents. + +Decision +======== + +1. Treat ``GET`` as strictly read-only for all REST APIs. +2. Move side-effect behavior out of ``GET`` handlers: + + * Create explicit write endpoints (``POST``, ``PUT``, ``PATCH``) for state changes. + * If telemetry must exist, decouple it using async event pipelines (emit events without + mutating domain state) and ensure API responses are not dependent on state mutation. + +3. Add regression tests to ensure ``GET`` handlers do not modify domain state. +4. Document exceptions (if any) and provide migration notes for clients. + +Relevance in edx-platform +========================= + +* **GET used with side-effects**: Various views use ``@require_GET`` while + triggering writes (e.g. tracking, first-access, or logging). Discussion views + (``lms/djangoapps/discussion/views.py``) use ``@require_GET`` for thread/topic + listing; any implicit tracking on read should be moved to separate endpoints or + async events. +* **Event emission on read**: ``common/djangoapps/student`` and courseware code + sometimes emit events (e.g. ``tracker.emit``, streak updates) in code paths + triggered by GET; these should be decoupled so GET handlers do not mutate + domain state. + +Code example +============ + +**Anti-pattern (GET that writes):** + +.. code-block:: python + + @require_GET + def get_progress(request, course_id): + # BAD: recording "first access" or analytics on every GET + record_first_access(request.user, course_id) + return JsonResponse(compute_progress(...)) + +**Preferred: read-only GET + optional separate track endpoint** + +.. code-block:: python + + @require_GET + def get_progress(request, course_id): + return Response(ProgressSerializer(compute_progress(...)).data) + + @require_POST + def track_progress_view(request, course_id): + # Or emit via async pipeline; response does not depend on write + emit_progress_viewed_event(request.user, course_id) + return Response(status=204) + +Consequences +============ + +* Pros + + * REST-compliant behavior; safer automated consumption (AI agents, integrations). + * Predictable caching/proxy semantics. + +* Cons / Costs + + * Requires refactoring legacy courseware/analytics endpoints that currently log on read. + * Potential behavior changes for internal analytics that relied on implicit GET-triggered writes. + +Implementation Notes +==================== + +* Inventory endpoints with GET side-effects. +* For each, define a read-only GET representation and a separate write/track endpoint (or async + event emission) if needed. + +References +========== + +* “Non-Idempotent GET Requests” recommendation in the Open edX REST API standardization notes. From f04ab2999867b2207ea9ad86e8c560792876d5fd Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed Date: Wed, 8 Apr 2026 11:06:07 +0500 Subject: [PATCH 2/2] docs: address feedback --- .../0030-ensure-get-requests-are-idempotent.rst | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/docs/decisions/0030-ensure-get-requests-are-idempotent.rst b/docs/decisions/0030-ensure-get-requests-are-idempotent.rst index 8bf1830eaa3d..f3e45d0b13f5 100644 --- a/docs/decisions/0030-ensure-get-requests-are-idempotent.rst +++ b/docs/decisions/0030-ensure-get-requests-are-idempotent.rst @@ -1,5 +1,5 @@ Ensure GET is Idempotent -=============================== +======================== :Status: Proposed :Date: 2026-03-31 @@ -15,12 +15,16 @@ caching/proxy behavior and automated clients/agents. Decision ======== -1. Treat ``GET`` as strictly read-only for all REST APIs. -2. Move side-effect behavior out of ``GET`` handlers: +1. Treat ``GET`` as strictly read-only with respect to **domain state**: a ``GET`` handler + must not create, update, or delete records that are part of the transactional domain model + (e.g. enrollments, grades, user profile fields). +2. Move domain-state-mutating side-effects out of ``GET`` handlers: * Create explicit write endpoints (``POST``, ``PUT``, ``PATCH``) for state changes. - * If telemetry must exist, decouple it using async event pipelines (emit events without - mutating domain state) and ensure API responses are not dependent on state mutation. + * Telemetry and analytics writes to a **separate analytics store** (e.g. event tracking, + segment events, read-count increments) are acceptable inside a ``GET`` handler provided + the response content does not depend on them. These writes do not need to be moved to + async pipelines unless there is a specific performance or reliability reason to do so. 3. Add regression tests to ensure ``GET`` handlers do not modify domain state. 4. Document exceptions (if any) and provide migration notes for clients.