• 
      

    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)