diff --git a/app/controllers/projects/compare_diffs_stream_controller.rb b/app/controllers/projects/compare_diffs_stream_controller.rb new file mode 100644 index 0000000000000000000000000000000000000000..1930400724bf2050975de53eb80e21ae3360e372 --- /dev/null +++ b/app/controllers/projects/compare_diffs_stream_controller.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Projects + class CompareDiffsStreamController < Projects::CompareController + include StreamDiffs + + private + + def resource + compare + end + end +end diff --git a/app/models/compare.rb b/app/models/compare.rb index 350149c08bf1d0bbe2793558be47d83ee9ff0fb4..a4523417fd930f8d0a93a63b2863d7ed5d800702 100644 --- a/app/models/compare.rb +++ b/app/models/compare.rb @@ -90,6 +90,8 @@ def diffs(diff_options = nil) diff_refs: diff_refs) end + alias_method :diffs_for_streaming, :diffs + def diff_refs Gitlab::Diff::DiffRefs.new( base_sha: @straight ? start_commit_sha : base_commit_sha, diff --git a/config/routes/repository.rb b/config/routes/repository.rb index 2efea11c268a79c6c64d42e04658d29a5fa045ef..a8808f69340cfefecfc69472d96b921cc66f3134 100644 --- a/config/routes/repository.rb +++ b/config/routes/repository.rb @@ -18,6 +18,7 @@ collection do get :diff_for_path get :signatures + get :diffs_stream, to: 'compare_diffs_stream#diffs' end end diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index d09252ff033b92b6362e24cbf622dd1349302b16..2323b489e4d3a2678d2f66cda61cadf82dfb4668 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -97,7 +97,7 @@ def between(repo, head, base, options = {}, *paths) # as generated. def filter_diff_options(options, default_options = {}) allowed_options = [:ignore_whitespace_change, :max_files, :max_lines, - :limits, :expanded, :collect_all_paths, :generated_files] + :limits, :expanded, :collect_all_paths, :generated_files, :offset_index] if default_options actual_defaults = default_options.dup diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 13c44afef2fd29225701fea4045ccc049b459a18..c04ad13953dad309e52732d8af55d556de9dbd86 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -354,7 +354,7 @@ end describe '.filter_diff_options' do - let(:options) { { max_files: 100, invalid_opt: true } } + let(:options) { { max_files: 100, invalid_opt: true, offset_index: 10 } } context "without default options" do let(:filtered_options) { described_class.filter_diff_options(options) } diff --git a/spec/requests/projects/commit_diffs_stream_controller_spec.rb b/spec/requests/projects/commit_diffs_stream_controller_spec.rb index d32c4f6c1b0e8b463a2e19d357aaf340add740bd..debd71b6841c281906307c1e81558f1d4de93200 100644 --- a/spec/requests/projects/commit_diffs_stream_controller_spec.rb +++ b/spec/requests/projects/commit_diffs_stream_controller_spec.rb @@ -7,6 +7,7 @@ let_it_be(:user) { create(:user, maintainer_of: project) } let(:commit_with_two_diffs) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e") } let(:offset) { 0 } + let(:diff_files) { commit_with_two_diffs.diffs.diff_files } before do sign_in(user) @@ -24,12 +25,6 @@ def send_request(**extra_params) get diffs_stream_namespace_project_commit_path(params.merge(extra_params)) end - it 'streams the response' do - send_request - - expect(response).to have_gitlab_http_status(:success) - end - it 'includes all diffs' do send_request @@ -40,53 +35,6 @@ def send_request(**extra_params) end end - context 'when offset is given' do - context 'when offset is 1' do - let(:offset) { 1 } - - it 'streams diffs except the offset' do - send_request - - diff_files = commit_with_two_diffs.diffs.diff_files.to_a - expect(response.body).not_to include(diff_files.first.new_path) - expect(response.body).to include(diff_files.last.new_path) - end - end - - context 'when offset is same as number of diffs' do - let(:offset) { commit_with_two_diffs.diffs.size } - - it 'no diffs are streamed' do - send_request - - expect(response.body).to be_empty - end - end - end - - context 'when an exception occurs' do - before do - allow(::RapidDiffs::DiffFileComponent) - .to receive(:new).and_raise(StandardError.new('something went wrong')) - end - - it 'prints out error message' do - send_request - - expect(response.body).to include('something went wrong') - end - end - - context 'when the rapid_diffs feature flag is disabled' do - before do - stub_feature_flags(rapid_diffs: false) - end - - it 'returns a 404 status' do - send_request - - expect(response).to have_gitlab_http_status(:not_found) - end - end + include_examples 'diffs stream tests' end end diff --git a/spec/requests/projects/compare_diffs_stream_controller_spec.rb b/spec/requests/projects/compare_diffs_stream_controller_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ae61785756b353883824572a114fdceab2daf91f --- /dev/null +++ b/spec/requests/projects/compare_diffs_stream_controller_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Compare diffs stream', feature_category: :source_code_management do + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:user) { create(:user, maintainer_of: project) } + + let(:whitespace) { nil } + let(:straight) { true } + let(:page) { nil } + let(:from_project_id) { nil } + let(:offset) { 0 } + let(:start_ref) { '08f22f25' } + let(:target_ref) { 'master' } + + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + from_project_id: from_project_id, + from: start_ref, + to: target_ref, + w: whitespace, + page: page, + straight: straight, + offset: offset + } + end + + let(:raw_compare) do + project.repository.compare_source_branch(target_ref, project.repository, start_ref, straight: straight) + end + + let(:compare) { Compare.new(raw_compare, project, base_sha: nil, straight: straight) } + let(:diff_files) { compare.diffs.diff_files } + + describe 'GET diffs_stream' do + def send_request(**extra_params) + get diffs_stream_namespace_project_compare_index_path(request_params.merge(extra_params)) + end + + it 'includes all diffs' do + send_request + + streamed_content = response.body + + diff_files.each do |diff_file| + expect(streamed_content).to include(diff_file.old_path) + end + end + + include_examples 'diffs stream tests' + end +end diff --git a/spec/support/shared_examples/diffs_stream_shared_examples.rb b/spec/support/shared_examples/diffs_stream_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..76f8aabd14c2835f8e6d4685d0e7c4686ca48f4b --- /dev/null +++ b/spec/support/shared_examples/diffs_stream_shared_examples.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'diffs stream tests' do + it 'streams the response' do + send_request + + expect(response).to have_gitlab_http_status(:success) + end + + context 'when offset is given' do + context 'when offset is 1' do + let(:offset) { 1 } + + it 'streams diffs except the offset' do + send_request + + diff_files_array = diff_files.to_a + expect(response.body).not_to include(diff_files_array.first.new_path) + expect(response.body).to include(diff_files_array.last.new_path) + end + end + + context 'when offset is the same as the number of diffs' do + let(:offset) { diff_files.size } + + it 'no diffs are streamed' do + send_request + + expect(response.body).to be_empty + end + end + end + + context 'when an exception occurs' do + before do + allow(::RapidDiffs::DiffFileComponent) + .to receive(:new).and_raise(StandardError.new('something went wrong')) + end + + it 'prints out error message' do + send_request + + expect(response.body).to include('something went wrong') + end + end + + context 'when the rapid_diffs feature flag is disabled' do + before do + stub_feature_flags(rapid_diffs: false) + end + + it 'returns a 404 status' do + send_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end +end