Wrong caching logic in ProcessRefChangesService

Hi,

in ProcessRefChangesService#merge_request_branches_for, the results are cached on an instance variable, but merge_request_branches_for is called by process_changes, itself working on changes that have been earlier grouped by action in process_changes_by_action.

In other words, this is a case of caching a method result irrespective of its parameter value, whereas it can be called with successive different values. This caching was introduced somewhere between GitLab 13.1 and 13.3

As a consequence, if a PostReceive contains both branch removals and ordinary updates, only the removals get processed (they are first in the iteration).

This condition is probably unfrequent with regular human Git usage, but it affected Heptapod immediately because branch deletion happens automatically on MR acceptance when source branch is a Mercurial topic (and that's an intrinsic trait of the VCS itself). This was spotted immediately by a functional test for stacked MRs, in which the second MR wasn't updated after the first got merged. I can imagine it to be also a problem in a Git context with automated bulk updates (mirrorring perhaps?).

In the case of Heptapod, the easy solution is to remove the caching, as I don't believe it'd had noticeable performance impact. I'm not sure if that is acceptable for upstream GitLab.

Don't hesitate to tell me what kind of reproduction would be interesting for that. Actually baking a pure Git example would probably cost me too much time, but I can submit a RSpec test.

Thanks!