Cache the review request's draft on the instance.
Review Request #14376 — Created March 19, 2025 and updated — Latest diff uploaded
ReviewRequest.get_draft()
fetched the draft every time it was called.
This was somewhat acceptable once, but with the introduction of actions
and some fields, we ended up calling this a lot, leading to an excess
of database queries on the review request pages. This also meant that
all the callers got separate instances, which could in theory cause some
problems with state synchronization and data loss during mutation.We now cache this on the
ReviewRequest
object, so that repeated
calls will return the same draft instance. We clear this when calling
ReviewRequest.clear_local_caches()
, which includes when the draft is
deleted or published.The codebase was audited to make sure this wouldn't result in any
obvious problems, though it does mean that callers that may be working
with multiple instances of the same review request (such as API unit
tests) must be careful to clear caches if re-querying for a draft.
All unit tests pass.
Spent some time running with this patch locally, testing the typical
functionality of posting changes, updating changes, publishing, deleting.
Never hit any issues. It will need more production testing before
release.
Diff Revision 3 (Latest)
Commits
Files
reviewboard/reviews/models/review_request.py |
---|
reviewboard/reviews/models/review_request_draft.py |
---|
reviewboard/reviews/tests/test_review_request.py |
---|
reviewboard/reviews/tests/test_review_request_detail_view.py |
---|
reviewboard/reviews/tests/test_review_request_page_data.py |
---|
reviewboard/reviews/tests/test_signal_handlers.py |
---|
reviewboard/webapi/resources/diff.py |
---|
reviewboard/webapi/resources/draft_diffcommit.py |
---|
reviewboard/webapi/resources/review_request_draft.py |
---|
reviewboard/webapi/tests/test_review_request_draft.py |
---|