Make optimistic locking work: ETags follow the record version
b64db27fix(customers)1 review thread
Illustrative: based on a real change, edited for clarity and privacy.
src/adapters/primary/fastapi/routes/address/get.py+5 −2
| @@ | |||
| 3 | 3 | from fastapi import APIRouter, Header, Response, status | |
| 4 | 4 | ||
| 5 | 5 | from src.adapters.primary.fastapi.etag import ( | |
| 6 | removed | generate_etag, | |
| 6 | added | stored_etag, | |
| 7 | 7 | validate_if_none_match, | |
| 8 | 8 | ) | |
| 9 | 9 | from src.adapters.primary.fastapi.models.address_attribute import ( | |
| @@ async def _get_attribute( | |||
| 126 | 128 | attr = self._fetch_attribute(customer_ref, attribute_ref) | |
| 127 | 129 | ||
| 128 | 130 | # Handle conditional GET with If-None-Match | |
| 129 | removed | current_etag = generate_etag(attr.updated_at, attr.ref) | |
| 131 | added | # The ETag is the row version, the same number If-Match is checked against | |
| 132 | added | # on writes, so a read and a later write can never disagree about staleness. | |
| 1 line 133, review thread 1 | added | current_etag = stored_etag(attr.version, attr.ref) | |
| 130 | 134 | if if_none_match is not None and validate_if_none_match( | |
| 131 | 135 | if_none_match, current_etag | |
| 132 | 136 | ): | |
| @@ async def _get_attribute( | |||
| 137 | 141 | ) | |
| 138 | 142 | ||
| 139 | 143 | # Build full response | |
| 144 | added | response.headers["ETag"] = current_etag | |
| 140 | 145 | return AddressAttributeResponse.from_dto(attr) | |
| 141 | 146 | ||
| 142 | 147 | async def _head_attribute( | |
AI reviewP1
Relationship changes keep old ETags
A PATCH that only changes an address label or the primary flag updates the relationship, not the attribute, but this ETag only uses the attribute’s version. A client can get a 304 for stale data, or make a later write with an old
If-Matchthat still passes.author
Fixed: a link change now marks the attribute as updated, so changing the label, primary flag or grouping moves its version and ETag. A new test covers a stale
If-Matchafter a label-only change, which now returns 412.reviewer
Nice. Can the 412 go in the API docs too, so client teams know to refetch and retry?
author
Done: the endpoint docs now describe the 412 and what to do about it.