RFR: 1530: Race with CheckWorkItem in PR bot

[Kevin Rushforth]

On Fri, 5 Aug 2022 21:49:18 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> This patch fixes a race in the PR bot. 
> 
> Currently, all WorkItems that process PRs get an instance of the PullRequest that was  fetched before the WorkItem instance was created. The data in that PullRequest instance (the parts that aren't dynamically fetched on method calls) may then be stale before the WorkItem even starts running.
> 
> With this patch, I'm changing PullRequestWorkItem (the super class of all WorkItems that handle PRs), to only be instantiated with the ID of the PR, and then fetches the actual PullRequest object at the start of the `run()` method. When the `run()` method is called, we are guaranteed to be the only WorkItem currently processing the particular PR, so fetching data there should guarantee a coherent and current view.
> 
> In many cases where PullRequestWorkItems were created, the creator first refreshed the PullRequest object to provide an updated view, reflecting any changes made by the creator. These refreshes are no longer necessary, so I've removed them all.

Looks like a good solution to me.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestWorkItem.java line 49:

> 47:             return true;
> 48:         }
> 49:         if (!(prId.equals(otherItem.prId) && bot.repo().isSame(otherItem.bot.repo()))) {

This means that two work requests can run concurrently if either the `prId` is different or the `bot repo` is different. I presume this is because the `prId` is relative to the repo (which makes sense), right?

-------------

Marked as reviewed by kcr (Reviewer).

PR: https://git.openjdk.org/skara/pull/1349