Fix storing and locating parent diff source information.
Review Request #11099 — Created July 27, 2020 and updated
This makes some important changes to how we store information on source
files referenced in parent diffs, accompanying information for the
primary diff, and how we look up files.
When parent diffs were added, we had some restrictions on our schema.
There was no
extra_data, and we only really had the source/destination
file/revision fields to work with. At that point, we opted to set the
source file/revision fields to point to the source of the parent diff,
so that we would fetch the right source file to apply the patch to.
This wasn't really ideal, as it sort of masked the actual revisions of
the modified file the reviewer was looking at. It was also broken -- we
didn't actually update the source filename, just the revision.
That latter issue is what led to this change in the first place. On Git
(which we first introduced parent diff support for), we didn't need that
filename, just the revision (the blob SHA), so a renamed parent file
didn't break anything.
In Mercurial, though, we need both the file and revision. Not having the
true original filename resulted in breakages when the file was
moved/renamed in the parent diff.
Going forward, we store the parent source filename/revision in
FileDiff.extra_data, and we keep the
FileDifffields specific to the
primary diff posted for review.
get_original_file()will attempt to locate using the parent source, if
available, and fall back on the main
FileDiffsource. This keeps
everything working as it always has for any existing diffs (and of
course for diffs without a parent diff), but allows us to maintain this
clean separation going forward.
Anyone with diffs in a database that broke due to a moved/renamed parent
source filename, who had other means of working around it, can simply
FileDiff.extra_datakeys to point to the correct location,
fixing the issue without having to re-upload diffs.
Unit tests pass.
Tested viewing existing diffs containing parent diffs, and didn't hit any
Posted new Mercurial and Git diffs with parent diffs, both with and
without files being moved. In all cases, the source file were correctly
fetched and the files rendered with the expected information.