Set MergeRequestDiff#external_diff_store in more cases
What does this MR do?
Merge request diffs can store their data either in-database, or in an external file. That file may be on disk (recommended against) or in object storage (enabled on GitLab.com today).
We've had a couple of reports where the merge_request_diff.external_diff_store
column has had the wrong value for some diffs. The practical outcome of this is that the MR "changes" tab cannot be displayed - the file is correctly stored in object storage, but since we don't record 2
in this column, we don't know to look it up from there.
This MR spots and remedies a couple of deficiencies in when we set this value.
Excluding it during project import is unnecessary, because importing a project actually persists all the MR diffs as in-database ones, that are then migrated to become external storage diffs. This is something we need to improve, so they go straight to object storage, but in the meantime, we can clean this up.
Recalculating every time stored_externally
is changed, in addition to external_diff
(this is just the filename) changing, seems to be the cleanest way to plug the gap.
On GitLab.com, and presumably on a range of other instances too, we now have records that need fixing - this can be done as a follow-up MR once this one is merged. We shouldn't close the associated issue until this is done.
Since we're trying to roll out external diffs on GitLab.com, per gitlab-com/gl-infra/production#1851 (closed) , we may want to Pick into auto-deploy to ensure this fix reaches GitLab.com sooner.
Screenshots
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Part of #216216 (closed)
Closes #197317 (closed)