A review that should be outdated can stay valid when several saves of the change request race with a concurrent document load

Symptom

When a review is posted on a change request, or a new change is saved in it, a review that should become outdated sometimes stays valid. The review count is right, only the "outdated" state of the previous review is lost, and it is lost in the stored data, not just in the display.

For example, an approver who first requested changes and then approves keeps a valid "Change requested" review next to the new approval. In the other direction, after a new change is saved, an approval can stay valid instead of being outdated. A valid approval is what makes a change request ready for publication, so it can be reported ready while one of its approvals should have been superseded. The race is more likely on a busy or slow server.

It makes the functional test DelegateApproversIT#delegateApprovalAndReview flicker on CI (3 of the 20 main builds 841 to 860):

• build 860, DelegateApproversIT.java:280: Bar approves for himself, his previous "Change requested" review stays valid.
• build 855, DelegateApproversIT.java:315: after a new change is saved, the approval given on behalf of Foo stays valid while the other approval is outdated.

CRAPP-438 fixed the same symptom in the change request storage cache. These failures happened after that fix, one layer below it.

Root cause

The application saves the change request document several times in a row in the same request: DefaultChangeRequestManager#addReview saves the new review, then the outdated previous review, then the change request, then possibly its new status. invalidateReviews followed by the status computation does the same. Each save clones the document from the XWiki document cache, changes the clone and saves it.

On XWiki 14.10 the document cache can be left with an older version of a document: XWIKI-23055 (fixed in 16.10.9 and 17.3.0-rc-1). A background thread loading the document from the database (e.g. the Solr indexer reacting to the first save) can put the version it loaded into the cache after one of our later saves removed it. The next save in the sequence then clones that older version and writes it back, which reverts the review invalidation saved just before.

Reproduction

Forced locally on 14.10.12 by replaying the XWIKI-23055 interleaving in addReview: load the change request document from the database store right after the new review is saved, and put it into the XWikiCacheStore cache right after the previous review is invalidated. DelegateApproversIT#delegateApprovalAndReview then fails at line 280 with the same assertion and the same screen as CI build 860.

Fix constraints

Upgrading the parent to a version that includes XWIKI-23055 is not an option yet: the application still needs to support XWiki 14.10. The fix has to be done in the application. For example, apply all the changes made for one action to a single document and save it once, instead of several consecutive clone-and-save sequences.