Skip to content

Commit 9f4a500

Browse files
Taimoor  AhmedTaimoor  Ahmed
authored andcommitted
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.
1 parent 2e2ebb9 commit 9f4a500

1 file changed

Lines changed: 92 additions & 0 deletions

File tree

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
===============================
2+
Ensure GET is Idempotent
3+
===============================
4+
5+
:Status: Proposed
6+
:Date: 2026-03-31
7+
:Deciders: API Working Group
8+
9+
Context
10+
=======
11+
12+
Some Open edX endpoints use ``GET`` requests that have side-effects (e.g., writing tracking logs,
13+
recording first access events). This violates REST safety/idempotency expectations and can break
14+
caching/proxy behavior and automated clients/agents.
15+
16+
Decision
17+
========
18+
19+
1. Treat ``GET`` as strictly read-only for all REST APIs.
20+
2. Move side-effect behavior out of ``GET`` handlers:
21+
22+
* Create explicit write endpoints (``POST``, ``PUT``, ``PATCH``) for state changes.
23+
* If telemetry must exist, decouple it using async event pipelines (emit events without
24+
mutating domain state) and ensure API responses are not dependent on state mutation.
25+
26+
3. Add regression tests to ensure ``GET`` handlers do not modify domain state.
27+
4. Document exceptions (if any) and provide migration notes for clients.
28+
29+
Relevance in edx-platform
30+
=========================
31+
32+
* **GET used with side-effects**: Various views use ``@require_GET`` while
33+
triggering writes (e.g. tracking, first-access, or logging). Discussion views
34+
(``lms/djangoapps/discussion/views.py``) use ``@require_GET`` for thread/topic
35+
listing; any implicit tracking on read should be moved to separate endpoints or
36+
async events.
37+
* **Event emission on read**: ``common/djangoapps/student`` and courseware code
38+
sometimes emit events (e.g. ``tracker.emit``, streak updates) in code paths
39+
triggered by GET; these should be decoupled so GET handlers do not mutate
40+
domain state.
41+
42+
Code example
43+
============
44+
45+
**Anti-pattern (GET that writes):**
46+
47+
.. code-block:: python
48+
49+
@require_GET
50+
def get_progress(request, course_id):
51+
# BAD: recording "first access" or analytics on every GET
52+
record_first_access(request.user, course_id)
53+
return JsonResponse(compute_progress(...))
54+
55+
**Preferred: read-only GET + optional separate track endpoint**
56+
57+
.. code-block:: python
58+
59+
@require_GET
60+
def get_progress(request, course_id):
61+
return Response(ProgressSerializer(compute_progress(...)).data)
62+
63+
@require_POST
64+
def track_progress_view(request, course_id):
65+
# Or emit via async pipeline; response does not depend on write
66+
emit_progress_viewed_event(request.user, course_id)
67+
return Response(status=204)
68+
69+
Consequences
70+
============
71+
72+
* Pros
73+
74+
* REST-compliant behavior; safer automated consumption (AI agents, integrations).
75+
* Predictable caching/proxy semantics.
76+
77+
* Cons / Costs
78+
79+
* Requires refactoring legacy courseware/analytics endpoints that currently log on read.
80+
* Potential behavior changes for internal analytics that relied on implicit GET-triggered writes.
81+
82+
Implementation Notes
83+
====================
84+
85+
* Inventory endpoints with GET side-effects.
86+
* For each, define a read-only GET representation and a separate write/track endpoint (or async
87+
event emission) if needed.
88+
89+
References
90+
==========
91+
92+
* “Non-Idempotent GET Requests” recommendation in the Open edX REST API standardization notes.

0 commit comments

Comments
 (0)