[docs] Initial draft of the REST style guide - #1649
Conversation
✅ Deploy Preview for moodledevdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Adds a new REST documentation section to the Moodle DevDocs site, including an initial (but intended-to-be-normative) REST API style guide to standardize URI structure, naming, payload conventions, and HTTP semantics across Moodle components.
Changes:
- Introduces a comprehensive REST API style guide covering resource design, URI conventions, JSON conventions, pagination, errors, and idempotency.
- Adds a
RESTguide index page underdocs/guides/rest/.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| docs/guides/rest/style.md | Adds the initial REST style guide content and examples. |
| docs/guides/rest/index.md | Adds the REST guides section index frontmatter (and should include navigational content). |
Comments suppressed due to low confidence (2)
docs/guides/rest/style.md:998
- This TODO note also uses broken Markdown emphasis ("-*TO DO:**"). Formatting it consistently will improve readability of the draft.
-*TO DO:** this section is under revision; most of it is being removed as incorrect, so its rationale will be drafted once the replacement content is agreed.
docs/guides/rest/index.md:7
docs/guides/rest/index.mdcurrently contains only frontmatter, so the rendered page will be empty. Adding a short introduction and links (e.g. to the style guide) will make the section navigable.
---
| GET /courses/{courseid} | ||
| POST /courses | ||
| PATCH /courses/{courseid} | ||
| DELETE /courses/{courseid} |
|
|
||
| ### 7.1 Nested Resources | ||
|
|
||
| Nested resources MUST only be used only for direct ownership or containment relationships. |
| { | ||
| "courseId": 42, | ||
| "shortName": "math101", | ||
| "createdAt": 1779193800, | ||
| "startDate": 1779193800 | ||
| } |
| "pagination": { | ||
| "page": 1, | ||
| "pagesize": 25, | ||
| "totalitems": 1 | ||
| } | ||
| } |
| "pagination": { | ||
| "page": 1, | ||
| "pagesize": 25, | ||
| "totalitems": 100 | ||
| } | ||
| } |
|
|
||
| #### RATIONALE | ||
|
|
||
| -*TO DO:** this rationale needs further work — the scope naming examples in this section are still under review and will be revised before this rationale is finalized. |
| - OAuth2 | ||
| - ****** Service tokens | ||
|
|
|
|
||
| #### RATIONALE | ||
|
|
||
| Declarative payloads describe what the resource should look like afterwards, which is idempotent, easy to validate against a schema, and safe to retry. Imperative payloads like "move": "up" encode a transition rather than a state, so retries or race conditions between two clients could produce different results depending on execution order, undermining PATCH's idempotency expectations from section 15.2. |
| /courses/{courseid}/sections | ||
| /mod_forum/It was /{discussionid}/posts/{postid} |
| | Canonical Patterns | reusable API patterns | | ||
| | Rationale | explanatory guidance | | ||
|
|
||
| Normative sections MUST use only: |
There was a problem hiding this comment.
Can we quote or reference RFC 2119 and 8174 here? I know, this is not an Internet Standard, but RFC 2119 was created for a reason, and IMHO it fits very well here, too.
| Normative sections MUST use only: | |
| Normative sections MUST use only the following key words, and they are to be interpreted as described in [BCP 14](https://datatracker.ietf.org/doc/html/bcp14) [[RFC2119](https://datatracker.ietf.org/doc/html/rfc2119)] [[RFC8174](https://datatracker.ietf.org/doc/html/rfc8174)] when, and only when, they appear in all capitals: |
|
|
||
| ## 3. Core REST Principles | ||
|
|
||
| All Moodle APIs MUST satisfy **Richardson Maturity Model Level 2**. |
There was a problem hiding this comment.
We should include a link to a normative source so that people unfamiliar with the model can read about it.
| All Moodle APIs MUST satisfy **Richardson Maturity Model Level 2**. | |
| All Moodle APIs MUST satisfy **[Richardson Maturity Model Level 2](https://martinfowler.com/articles/richardsonMaturityModel.html)**. |
| | 404 | Resource not found | | ||
| | 409 | Conflict | | ||
| | 422 | Validation failure | |
There was a problem hiding this comment.
Section 15.1 also mentions 412 Precondition Failed, so we should include it here.
| | 404 | Resource not found | | |
| | 409 | Conflict | | |
| | 422 | Validation failure | | |
| | 404 | Resource not found | | |
| | 409 | Conflict | | |
| | 412 | Precondition Failed | | |
| | 422 | Validation failure | |
| | PUT | Yes | | ||
| | DELETE | Yes | | ||
| | POST | No | | ||
| | PATCH | No guarantee | |
There was a problem hiding this comment.
Minor: Can we simplify PATCH just "No" instead of "No guarantee"?
| | POST | No | | ||
| | PATCH | No guarantee | | ||
|
|
||
| Retryable POST operations SHOULD support: |
There was a problem hiding this comment.
Thoughts about upgrading Idempotency-Key from SHOULD to MUST?
- This would make retry safety a first principle. See my other comment below.
- In general, fewer optionality reduces ambiguity and thus complexity.
Of course, the downside is slightly more client-side complexity. But in my opinion is well worth not having to deal with support calls about duplicate records due to retries.
|
|
||
| #### RATIONALE | ||
|
|
||
| Clients on unreliable networks, particularly mobile, need to safely retry requests without accidentally creating duplicate resources; since POST is inherently non-idempotent, an explicit Idempotency-Key lets the server recognize and deduplicate a retried creation request as the same logical operation, rather than the client having to guess whether a prior request actually succeeded before retrying. |
There was a problem hiding this comment.
It's not just network unreliability but rather unreliability in general, which would include server overload, especially during the start of semester (leading to duplicate requests).
jtse
left a comment
jtse
left a comment
There was a problem hiding this comment.
Thanks for writing this! A general point: Consider having fewer SHOULDs, either upgrading them to MUST or removing them. SHOULD in specs tend to make things more complicated.
| Rules: | ||
|
|
||
| - must represent clear hierarchical ownership | ||
| - must remain shallow (maximum recommended depth: 2) |
There was a problem hiding this comment.
+1 to shallow resources (especially with respect to the technical reasons you outlined below). Where would a common resource like student submissions for an activity (assignment) go? If it's nested under activities it would be a depth of 3.
On the other hand, that nesting also feels intuitive to me as a dev. Maybe one way to resolve this is the top level resource courses (and enrollment) don't change often especially later in the school year/semester, so it's highly cacheable.
Thoughts?
jtse
left a comment
jtse
left a comment
There was a problem hiding this comment.
Looking forward to seeing how this evolves and prototype implementations.
This is the initial draft of the REST style guide for RFC.