Fix building the user manual.

Review Request #13948 — Created June 5, 2024 and submitted

Information

Review Board
release-7.x

Reviewers

Building the user manual was hitting a problem when trying to
serialize links for the DraftFileAttachmentResource, where it wasn't
finding the right review request.

After a trip far down the rabbit hole, I narrowed this down to a bug in
djblets.webapi.resources.base.WebAPIResource.get_object, and the caching
that it does. This method attempts to keep a cache of objects, which is
stored on the request object. Unfortunately, the cache key computation
is broken for resources which are singleton but are children of
non-singleton resources. In this case, ReviewRequestDraftResource is
marked as singleton, but its parent resource is not singleton. We really
ought to be chaining up the tree of resources and letting each one
contribute to the cache key.

Because changing the cache behavior in djblets is very risky right now
(even with a known bug of this magnitude), we're going to work around
this entirely in the docs code. This code exposed the bug because it
uses the same request object for everything. In order to work around it,
we create new requests for each get_queryset() call as we iterate
through the resource tree.

While I was here I fixed up some problems I found in the fixtures with
the file attachment and screenshot counts, where the counts had somehow
ended up with the values typed twice (33 instead of 3, for example).

Built user manual.

Summary ID
Fix building the user manual.
Building the user manual was hitting a problem when trying to serialize links for the DraftFileAttachmentResource, where it wasn't finding the right review request. After a trip far down the rabbit hole, I narrowed this down to a bug in djblets.webapi.resources.base.WebAPIResource.get_object, and the caching that it does. This method attempts to keep a cache of objects, which is stored on the request object. Unfortunately, the cache key computation is broken for resources which are singleton but are children of non-singleton resources. In this case, ReviewRequestDraftResource is marked as singleton, but its parent resource is not singleton. We really ought to be chaining up the tree of resources and letting each one contribute to the cache key. Because changing the cache behavior in djblets is very risky right now (even with a known bug of this magnitude), we're going to work around this entirely in the docs code. This code exposed the bug because it uses the same request object for everything. In order to work around it, we create new requests for each `get_queryset()` call as we iterate through the resource tree. While I was here I fixed up some problems I found in the fixtures with the file attachment and screenshot counts, where the counts had somehow ended up with the values typed twice (33 instead of 3, for example). Testing Done: Built user manual.
9a895f692c5762ad919706f4607694a163322d5c
chipx86
  1. 
      
  2. docs/manual/_ext/webapidocs.py (Diff revision 1)
     
     
     

    I'd argue it's more correct and realistic here, assuming this is mirroring what happens in real-world use. Since data gets stuffed onto a request in places, we probably never want to behave like we're accessing multiple APIs while assuming we can get away with a single request instance.

  3. 
      
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-7.x (32b6443)