Fix MR diffs created with gitaly_diff_between enabled
What does this MR do?
Fix MR diffs created with gitaly_diff_between enabled
When we save merge request diffs to the database, we need to expand the diff before doing so. That's so that we can expand diffs (within the normal limits) without hitting the repository, but just by going to the database.
This is done implicitly - diffs are expanded unless we say otherwise. However, we have another option we can pass, that lets us enforce diff size limits, that defaults to true.
Are there points in the code the reviewer needs to double check?
Another option for setting request_params[:collapse_diffs]
could be:
# Fetch from options without setting a default value
request_params[:collapse_diffs] = !!options[:enforce_limits] || !options.fetch(:expanded, true)
But it's not clear to me why these two were coupled together in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14251 initially, as they are not in Gitlab::Git::DiffCollection
.
Why was this MR needed?
Prior to this commit:
- The Rugged code path defaulted to setting
expanded: true
andenforce_limits: true
. - The Gitaly code path defaulted to setting
expanded: false
andenforce_limits: true
.
This was introduced by eb36fa17, which
implemented the initial feature. Since then, if the gitaly_diff_between
feature flag was enabled, MRs would have diffs that could not be expanded in
some cases, with no fix other than to disable the feature flag and force push to
the MR to refresh the diff in the database.
Screenshots (if relevant)
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Tests added for this feature/bug - Conform by the code review guidelines
-
Has been reviewed by a Backend maintainer
-
What are the relevant issue numbers?
Closes gitaly#1256 (closed).