diff options
| -rw-r--r-- | GITALY_SERVER_VERSION | 2 | ||||
| -rw-r--r-- | app/assets/javascripts/dispatcher.js | 4 | ||||
| -rw-r--r-- | app/assets/javascripts/helpers/user_feature_helper.js | 12 | ||||
| -rw-r--r-- | app/models/blob_viewer/composer_json.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/36041-notification-title.yml | 4 | ||||
| -rw-r--r-- | changelogs/unreleased/36611-error-in-getcomposer-link.yml | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/tc-git-tower-pagination-links.yml | 5 | ||||
| -rw-r--r-- | lib/api/helpers/pagination.rb | 17 | ||||
| -rw-r--r-- | lib/gitlab/git/repository.rb | 68 | ||||
| -rw-r--r-- | lib/gitlab/gitaly_client/repository_service.rb | 5 | ||||
| -rw-r--r-- | lib/gitlab/string_range_marker.rb | 34 | ||||
| -rw-r--r-- | spec/helpers/diff_helper_spec.rb | 8 | ||||
| -rw-r--r-- | spec/lib/api/helpers/pagination_spec.rb | 52 | ||||
| -rw-r--r-- | spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb | 6 | ||||
| -rw-r--r-- | spec/lib/gitlab/diff/inline_diff_marker_spec.rb | 14 | ||||
| -rw-r--r-- | spec/lib/gitlab/gitaly_client/repository_service_spec.rb | 76 | ||||
| -rw-r--r-- | spec/lib/gitlab/string_range_marker_spec.rb | 39 |
17 files changed, 262 insertions, 91 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c25c8e5b741..26bea73e811 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.30.0 +0.31.0 diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index a0ed5c23ffe..2bba7f55de1 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -414,7 +414,7 @@ import initChangesDropdown from './init_changes_dropdown'; case 'projects:tree:show': shortcut_handler = new ShortcutsNavigation(); - if (UserFeatureHelper.isNewRepo()) break; + if (UserFeatureHelper.isNewRepoEnabled()) break; new TreeView(); new BlobViewer(); @@ -434,7 +434,7 @@ import initChangesDropdown from './init_changes_dropdown'; shortcut_handler = true; break; case 'projects:blob:show': - if (UserFeatureHelper.isNewRepo()) break; + if (UserFeatureHelper.isNewRepoEnabled()) break; new BlobViewer(); initBlob(); break; diff --git a/app/assets/javascripts/helpers/user_feature_helper.js b/app/assets/javascripts/helpers/user_feature_helper.js index fcd8569819c..638118a5204 100644 --- a/app/assets/javascripts/helpers/user_feature_helper.js +++ b/app/assets/javascripts/helpers/user_feature_helper.js @@ -1,11 +1,7 @@ import Cookies from 'js-cookie'; -function isNewRepo() { - return Cookies.get('new_repo') === 'true'; -} - -const UserFeatureHelper = { - isNewRepo, +export default { + isNewRepoEnabled() { + return Cookies.get('new_repo') === 'true'; + }, }; - -export default UserFeatureHelper; diff --git a/app/models/blob_viewer/composer_json.rb b/app/models/blob_viewer/composer_json.rb index ef8b4aef8e8..def4879fbb5 100644 --- a/app/models/blob_viewer/composer_json.rb +++ b/app/models/blob_viewer/composer_json.rb @@ -9,7 +9,7 @@ module BlobViewer end def manager_url - 'https://getcomposer.com/' + 'https://getcomposer.org/' end def package_name diff --git a/changelogs/unreleased/36041-notification-title.yml b/changelogs/unreleased/36041-notification-title.yml new file mode 100644 index 00000000000..7c5e0a0cd0d --- /dev/null +++ b/changelogs/unreleased/36041-notification-title.yml @@ -0,0 +1,4 @@ +--- +title: Don't escape html entities in InlineDiffMarkdownMarker +merge_request: +author: diff --git a/changelogs/unreleased/36611-error-in-getcomposer-link.yml b/changelogs/unreleased/36611-error-in-getcomposer-link.yml new file mode 100644 index 00000000000..1ff6ec01684 --- /dev/null +++ b/changelogs/unreleased/36611-error-in-getcomposer-link.yml @@ -0,0 +1,5 @@ +--- +title: Fix external link to Composer website +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/tc-git-tower-pagination-links.yml b/changelogs/unreleased/tc-git-tower-pagination-links.yml new file mode 100644 index 00000000000..b99ef8c3c4c --- /dev/null +++ b/changelogs/unreleased/tc-git-tower-pagination-links.yml @@ -0,0 +1,5 @@ +--- +title: Improve API pagination headers when no record found +merge_request: 13629 +author: Jordan Patterson +type: fixed diff --git a/lib/api/helpers/pagination.rb b/lib/api/helpers/pagination.rb index 0764b58fb4c..95108292aac 100644 --- a/lib/api/helpers/pagination.rb +++ b/lib/api/helpers/pagination.rb @@ -11,7 +11,7 @@ module API def add_pagination_headers(paginated_data) header 'X-Total', paginated_data.total_count.to_s - header 'X-Total-Pages', paginated_data.total_pages.to_s + header 'X-Total-Pages', total_pages(paginated_data).to_s header 'X-Per-Page', paginated_data.limit_value.to_s header 'X-Page', paginated_data.current_page.to_s header 'X-Next-Page', paginated_data.next_page.to_s @@ -26,20 +26,25 @@ module API links = [] - request_params[:page] = paginated_data.current_page - 1 - links << %(<#{request_url}?#{request_params.to_query}>; rel="prev") unless paginated_data.first_page? + request_params[:page] = paginated_data.prev_page + links << %(<#{request_url}?#{request_params.to_query}>; rel="prev") if request_params[:page] - request_params[:page] = paginated_data.current_page + 1 - links << %(<#{request_url}?#{request_params.to_query}>; rel="next") unless paginated_data.last_page? + request_params[:page] = paginated_data.next_page + links << %(<#{request_url}?#{request_params.to_query}>; rel="next") if request_params[:page] request_params[:page] = 1 links << %(<#{request_url}?#{request_params.to_query}>; rel="first") - request_params[:page] = paginated_data.total_pages + request_params[:page] = total_pages(paginated_data) links << %(<#{request_url}?#{request_params.to_query}>; rel="last") links.join(', ') end + + def total_pages(paginated_data) + # Ensure there is in total at least 1 page + [paginated_data.total_pages, 1].max + end end end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 1d5ca68137a..aef7ae659fe 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -653,33 +653,15 @@ module Gitlab # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/328 def copy_gitattributes(ref) - begin - commit = lookup(ref) - rescue Rugged::ReferenceError - raise InvalidRef.new("Ref #{ref} is invalid") - end - - # Create the paths - info_dir_path = File.join(path, 'info') - info_attributes_path = File.join(info_dir_path, 'attributes') - - begin - # Retrieve the contents of the blob - gitattributes_content = blob_content(commit, '.gitattributes') - rescue InvalidBlobName - # No .gitattributes found. Should now remove any info/attributes and return - File.delete(info_attributes_path) if File.exist?(info_attributes_path) - return - end - - # Create the info directory if needed - Dir.mkdir(info_dir_path) unless File.directory?(info_dir_path) - - # Write the contents of the .gitattributes file to info/attributes - # Use binary mode to prevent Rails from converting ASCII-8BIT to UTF-8 - File.open(info_attributes_path, "wb") do |file| - file.write(gitattributes_content) + Gitlab::GitalyClient.migrate(:apply_gitattributes) do |is_enabled| + if is_enabled + gitaly_copy_gitattributes(ref) + else + rugged_copy_gitattributes(ref) + end end + rescue GRPC::InvalidArgument + raise InvalidRef end # Returns the Git attributes for the given file path. @@ -1012,6 +994,40 @@ module Gitlab raw_output.compact end + + def gitaly_copy_gitattributes(revision) + gitaly_repository_client.apply_gitattributes(revision) + end + + def rugged_copy_gitattributes(ref) + begin + commit = lookup(ref) + rescue Rugged::ReferenceError + raise InvalidRef.new("Ref #{ref} is invalid") + end + + # Create the paths + info_dir_path = File.join(path, 'info') + info_attributes_path = File.join(info_dir_path, 'attributes') + + begin + # Retrieve the contents of the blob + gitattributes_content = blob_content(commit, '.gitattributes') + rescue InvalidBlobName + # No .gitattributes found. Should now remove any info/attributes and return + File.delete(info_attributes_path) if File.exist?(info_attributes_path) + return + end + + # Create the info directory if needed + Dir.mkdir(info_dir_path) unless File.directory?(info_dir_path) + + # Write the contents of the .gitattributes file to info/attributes + # Use binary mode to prevent Rails from converting ASCII-8BIT to UTF-8 + File.open(info_attributes_path, "wb") do |file| + file.write(gitattributes_content) + end + end end end end diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index 6ad97e62941..a74a6dc6e78 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -32,6 +32,11 @@ module Gitlab request = Gitaly::RepositorySizeRequest.new(repository: @gitaly_repo) GitalyClient.call(@storage, :repository_service, :repository_size, request).size end + + def apply_gitattributes(revision) + request = Gitaly::ApplyGitattributesRequest.new(repository: @gitaly_repo, revision: revision) + GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request) + end end end end diff --git a/lib/gitlab/string_range_marker.rb b/lib/gitlab/string_range_marker.rb index 94fba0a221a..11aeec1ebfa 100644 --- a/lib/gitlab/string_range_marker.rb +++ b/lib/gitlab/string_range_marker.rb @@ -1,21 +1,31 @@ module Gitlab class StringRangeMarker - attr_accessor :raw_line, :rich_line - - def initialize(raw_line, rich_line = raw_line) - @raw_line = raw_line - @rich_line = ERB::Util.html_escape(rich_line) + attr_accessor :raw_line, :rich_line, :html_escaped + + def initialize(raw_line, rich_line = nil) + @raw_line = raw_line.dup + if rich_line.nil? + @rich_line = raw_line.dup + @html_escaped = false + else + @rich_line = ERB::Util.html_escape(rich_line) + @html_escaped = true + end end def mark(marker_ranges) return rich_line unless marker_ranges - rich_marker_ranges = [] - marker_ranges.each do |range| - # Map the inline-diff range based on the raw line to character positions in the rich line - rich_positions = position_mapping[range].flatten - # Turn the array of character positions into ranges - rich_marker_ranges.concat(collapse_ranges(rich_positions)) + if html_escaped + rich_marker_ranges = [] + marker_ranges.each do |range| + # Map the inline-diff range based on the raw line to character positions in the rich line + rich_positions = position_mapping[range].flatten + # Turn the array of character positions into ranges + rich_marker_ranges.concat(collapse_ranges(rich_positions)) + end + else + rich_marker_ranges = marker_ranges end offset = 0 @@ -31,7 +41,7 @@ module Gitlab offset += text.length - original_text.length end - rich_line.html_safe + @html_escaped ? rich_line.html_safe : rich_line end private diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index f81a9b6492c..0deea0ff6a3 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -135,10 +135,10 @@ describe DiffHelper do it "returns strings with marked inline diffs" do marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line) - expect(marked_old_line).to eq(%q{abc <span class="idiff left right deletion">'def'</span>}) - expect(marked_old_line).to be_html_safe - expect(marked_new_line).to eq(%q{abc <span class="idiff left right addition">"def"</span>}) - expect(marked_new_line).to be_html_safe + expect(marked_old_line).to eq(%q{abc <span class="idiff left right deletion">'def'</span>}) + expect(marked_old_line).not_to be_html_safe + expect(marked_new_line).to eq(%q{abc <span class="idiff left right addition">"def"</span>}) + expect(marked_new_line).not_to be_html_safe end end diff --git a/spec/lib/api/helpers/pagination_spec.rb b/spec/lib/api/helpers/pagination_spec.rb index fb3ef04b860..59deca7757b 100644 --- a/spec/lib/api/helpers/pagination_spec.rb +++ b/spec/lib/api/helpers/pagination_spec.rb @@ -52,7 +52,13 @@ describe API::Helpers::Pagination do expect_header('X-Page', '1') expect_header('X-Next-Page', '2') expect_header('X-Prev-Page', '') - expect_header('Link', any_args) + + expect_header('Link', anything) do |_key, val| + expect(val).to include('rel="first"') + expect(val).to include('rel="last"') + expect(val).to include('rel="next"') + expect(val).not_to include('rel="prev"') + end subject.paginate(resource) end @@ -75,15 +81,53 @@ describe API::Helpers::Pagination do expect_header('X-Page', '2') expect_header('X-Next-Page', '') expect_header('X-Prev-Page', '1') - expect_header('Link', any_args) + + expect_header('Link', anything) do |_key, val| + expect(val).to include('rel="first"') + expect(val).to include('rel="last"') + expect(val).to include('rel="prev"') + expect(val).not_to include('rel="next"') + end + + subject.paginate(resource) + end + end + end + + context 'when resource empty' do + describe 'first page' do + before do + allow(subject).to receive(:params) + .and_return({ page: 1, per_page: 2 }) + end + + it 'returns appropriate amount of resources' do + expect(subject.paginate(resource).count).to eq 0 + end + + it 'adds appropriate headers' do + expect_header('X-Total', '0') + expect_header('X-Total-Pages', '1') + expect_header('X-Per-Page', '2') + expect_header('X-Page', '1') + expect_header('X-Next-Page', '') + expect_header('X-Prev-Page', '') + + expect_header('Link', anything) do |_key, val| + expect(val).to include('rel="first"') + expect(val).to include('rel="last"') + expect(val).not_to include('rel="prev"') + expect(val).not_to include('rel="next"') + expect(val).not_to include('page=0') + end subject.paginate(resource) end end end - def expect_header(name, value) - expect(subject).to receive(:header).with(name, value) + def expect_header(*args, &block) + expect(subject).to receive(:header).with(*args, &block) end def expect_message(method) diff --git a/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb b/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb index 046b096e366..7e17437fa2a 100644 --- a/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb @@ -6,9 +6,9 @@ describe Gitlab::Diff::InlineDiffMarkdownMarker do let(:inline_diffs) { [2..5] } let(:subject) { described_class.new(raw).mark(inline_diffs, mode: :deletion) } - it 'marks the range' do - expect(subject).to eq("ab{-c 'd-}ef'") - expect(subject).to be_html_safe + it 'does not escape html etities and marks the range' do + expect(subject).to eq("ab{-c 'd-}ef'") + expect(subject).not_to be_html_safe end end end diff --git a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb index c3bf34c24ae..7296bbf5df3 100644 --- a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb @@ -2,11 +2,13 @@ require 'spec_helper' describe Gitlab::Diff::InlineDiffMarker do describe '#mark' do + let(:inline_diffs) { [2..5] } + let(:raw) { "abc 'def'" } + + subject { described_class.new(raw, rich).mark(inline_diffs) } + context "when the rich text is html safe" do - let(:raw) { "abc 'def'" } let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">'def'</span>}.html_safe } - let(:inline_diffs) { [2..5] } - let(:subject) { described_class.new(raw, rich).mark(inline_diffs) } it 'marks the range' do expect(subject).to eq(%{<span class="abc">ab<span class="idiff left">c</span></span><span class="space"><span class="idiff"> </span></span><span class="def"><span class="idiff right">'d</span>ef'</span>}) @@ -15,12 +17,10 @@ describe Gitlab::Diff::InlineDiffMarker do end context "when the text text is not html safe" do - let(:raw) { "abc 'def'" } - let(:inline_diffs) { [2..5] } - let(:subject) { described_class.new(raw).mark(inline_diffs) } + let(:rich) { "abc 'def' differs" } it 'marks the range' do - expect(subject).to eq(%{ab<span class="idiff left right">c 'd</span>ef'}) + expect(subject).to eq(%{ab<span class="idiff left right">c 'd</span>ef' differs}) expect(subject).to be_html_safe end end diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb new file mode 100644 index 00000000000..fd5f984601e --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +describe Gitlab::GitalyClient::RepositoryService do + let(:project) { create(:project) } + let(:storage_name) { project.repository_storage } + let(:relative_path) { project.disk_path + '.git' } + let(:client) { described_class.new(project.repository) } + + describe '#exists?' do + it 'sends a repository_exists message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:repository_exists) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(double(exists: true)) + + client.exists? + end + end + + describe '#garbage_collect' do + it 'sends a garbage_collect message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:garbage_collect) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(double(:garbage_collect_response)) + + client.garbage_collect(true) + end + end + + describe '#repack_full' do + it 'sends a repack_full message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:repack_full) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(double(:repack_full_response)) + + client.repack_full(true) + end + end + + describe '#repack_incremental' do + it 'sends a repack_incremental message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:repack_incremental) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(double(:repack_incremental_response)) + + client.repack_incremental + end + end + + describe '#repository_size' do + it 'sends a repository_size message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:repository_size) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(size: 0) + + client.repository_size + end + end + + describe '#apply_gitattributes' do + let(:revision) { 'master' } + + it 'sends an apply_gitattributes message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:apply_gitattributes) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(double(:apply_gitattributes_response)) + + client.apply_gitattributes(revision) + end + end +end diff --git a/spec/lib/gitlab/string_range_marker_spec.rb b/spec/lib/gitlab/string_range_marker_spec.rb index abeaa7f0ddb..6bc02459dbd 100644 --- a/spec/lib/gitlab/string_range_marker_spec.rb +++ b/spec/lib/gitlab/string_range_marker_spec.rb @@ -2,34 +2,39 @@ require 'spec_helper' describe Gitlab::StringRangeMarker do describe '#mark' do + def mark_diff(rich = nil) + raw = 'abc <def>' + inline_diffs = [2..5] + + described_class.new(raw, rich).mark(inline_diffs) do |text, left:, right:| + "LEFT#{text}RIGHT" + end + end + context "when the rich text is html safe" do - let(:raw) { "abc <def>" } let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def"><def></span>}.html_safe } - let(:inline_diffs) { [2..5] } - subject do - described_class.new(raw, rich).mark(inline_diffs) do |text, left:, right:| - "LEFT#{text}RIGHT" - end - end it 'marks the inline diffs' do - expect(subject).to eq(%{<span class="abc">abLEFTcRIGHT</span><span class="space">LEFT RIGHT</span><span class="def">LEFT<dRIGHTef></span>}) - expect(subject).to be_html_safe + expect(mark_diff(rich)).to eq(%{<span class="abc">abLEFTcRIGHT</span><span class="space">LEFT RIGHT</span><span class="def">LEFT<dRIGHTef></span>}) + expect(mark_diff(rich)).to be_html_safe end end context "when the rich text is not html safe" do - let(:raw) { "abc <def>" } - let(:inline_diffs) { [2..5] } - subject do - described_class.new(raw).mark(inline_diffs) do |text, left:, right:| - "LEFT#{text}RIGHT" + context 'when rich text equals raw text' do + it 'marks the inline diffs' do + expect(mark_diff).to eq(%{abLEFTc <dRIGHTef>}) + expect(mark_diff).not_to be_html_safe end end - it 'marks the inline diffs' do - expect(subject).to eq(%{abLEFTc <dRIGHTef>}) - expect(subject).to be_html_safe + context 'when rich text doeas not equal raw text' do + let(:rich) { "abc <def> differs" } + + it 'marks the inline diffs' do + expect(mark_diff(rich)).to eq(%{abLEFTc <dRIGHTef> differs}) + expect(mark_diff(rich)).to be_html_safe + end end end end |