Skip to content

Commit e581b94

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 e581b94

1 file changed

Lines changed: 91 additions & 0 deletions

File tree

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

0 commit comments

Comments
 (0)