diff options
32 files changed, 352 insertions, 104 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e6aa5559d99..45f1638f871 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -84,7 +84,7 @@ stages: - JOB_NAME=( $CI_JOB_NAME ) - export CI_NODE_INDEX=${JOB_NAME[-2]} - export CI_NODE_TOTAL=${JOB_NAME[-1]} - - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_${JOB_NAME[1]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json + - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json - export KNAPSACK_GENERATE_REPORT=true - export CACHE_CLASSES=true - cp ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} ${KNAPSACK_REPORT_PATH} @@ -115,7 +115,7 @@ stages: - JOB_NAME=( $CI_JOB_NAME ) - export CI_NODE_INDEX=${JOB_NAME[-2]} - export CI_NODE_TOTAL=${JOB_NAME[-1]} - - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_${JOB_NAME[1]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json + - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json - export KNAPSACK_GENERATE_REPORT=true - export CACHE_CLASSES=true - cp ${KNAPSACK_SPINACH_SUITE_REPORT_PATH} ${KNAPSACK_REPORT_PATH} @@ -178,8 +178,8 @@ update-knapsack: <<: *only-canonical-masters stage: post-test script: - - scripts/merge-reports ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/rspec_pg_node_*.json - - scripts/merge-reports ${KNAPSACK_SPINACH_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/spinach_pg_node_*.json + - scripts/merge-reports ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/rspec-pg_node_*.json + - scripts/merge-reports ${KNAPSACK_SPINACH_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/spinach-pg_node_*.json - '[[ -z ${KNAPSACK_S3_BUCKET} ]] || scripts/sync-reports put $KNAPSACK_S3_BUCKET $KNAPSACK_RSPEC_SUITE_REPORT_PATH $KNAPSACK_SPINACH_SUITE_REPORT_PATH' - rm -f knapsack/${CI_PROJECT_NAME}/*_node_*.json diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 37822dac064..22032d0f914 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -288,7 +288,11 @@ import BlobForkSuggestion from './blob/blob_fork_suggestion'; if (anchor) { const notesContent = anchor.closest('.notes_content'); const lineType = notesContent.hasClass('new') ? 'new' : 'old'; - notes.addDiffNote(anchor, lineType, false); + notes.toggleDiffNote({ + target: anchor, + lineType, + forceShow: true, + }); anchor[0].scrollIntoView(); // We have multiple elements on the page with `#note_xxx` // (discussion and diff tabs) and `:target` only applies to the first diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index e960f1e8018..67df1c0c4d7 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -860,10 +860,19 @@ const normalizeNewlines = function(str) { e.preventDefault(); const $link = $(e.currentTarget || e.target); const showReplyInput = !$link.hasClass('js-diff-comment-avatar'); - this.addDiffNote($link, $link.data('lineType'), showReplyInput); + this.toggleDiffNote({ + target: $link, + lineType: $link.data('lineType'), + showReplyInput + }); }; - Notes.prototype.addDiffNote = function(target, lineType, showReplyInput) { + Notes.prototype.toggleDiffNote = function({ + target, + lineType, + forceShow, + showReplyInput = false, + }) { var $link, addForm, hasNotes, newForm, noteForm, replyButton, row, rowCssToAdd, targetContent, isDiffCommentAvatar; $link = $(target); row = $link.closest("tr"); @@ -908,12 +917,12 @@ const normalizeNewlines = function(str) { notesContent = targetRow.find(notesContentSelector); addForm = true; } else { - targetRow.show(); - notesContent.toggle(!notesContent.is(':visible')); + const isCurrentlyShown = targetRow.find('.content:not(:empty)').is(':visible'); + const isForced = forceShow === true || forceShow === false; + const showNow = forceShow === true || (!isCurrentlyShown && !isForced); - if (!targetRow.find('.content:not(:empty)').is(':visible')) { - targetRow.hide(); - } + targetRow.toggle(showNow); + notesContent.toggle(showNow); } if (addForm) { diff --git a/app/models/namespace.rb b/app/models/namespace.rb index a7ede5e3b9e..4d59267f71d 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -46,7 +46,7 @@ class Namespace < ActiveRecord::Base before_destroy(prepend: true) { prepare_for_destroy } after_destroy :rm_dir - scope :root, -> { where('type IS NULL') } + scope :for_user, -> { where('type IS NULL') } scope :with_statistics, -> do joins('LEFT JOIN project_statistics ps ON ps.namespace_id = namespaces.id') diff --git a/app/models/repository.rb b/app/models/repository.rb index 9153e5ae5a8..07e0b3bae4f 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -649,22 +649,8 @@ class Repository "#{name}-#{highest_branch_id + 1}" end - # Remove archives older than 2 hours def branches_sorted_by(value) - case value - when 'name' - branches.sort_by(&:name) - when 'updated_desc' - branches.sort do |a, b| - commit(b.dereferenced_target).committed_date <=> commit(a.dereferenced_target).committed_date - end - when 'updated_asc' - branches.sort do |a, b| - commit(a.dereferenced_target).committed_date <=> commit(b.dereferenced_target).committed_date - end - else - branches - end + raw_repository.local_branches(sort_by: value) end def tags_sorted_by(value) diff --git a/app/models/user.rb b/app/models/user.rb index 088a7cb83d5..d63641891fd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -350,7 +350,7 @@ class User < ActiveRecord::Base end def find_by_full_path(path, follow_redirects: false) - namespace = Namespace.find_by_full_path(path, follow_redirects: follow_redirects) + namespace = Namespace.for_user.find_by_full_path(path, follow_redirects: follow_redirects) namespace&.owner end diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb index 0f94504625a..f846d72498f 100644 --- a/app/services/members/authorized_destroy_service.rb +++ b/app/services/members/authorized_destroy_service.rb @@ -10,7 +10,7 @@ module Members return false if member.is_a?(GroupMember) && member.source.last_owner?(member.user) Member.transaction do - unassign_issues_and_merge_requests(member) + unassign_issues_and_merge_requests(member) unless member.invite? member.destroy end diff --git a/app/views/projects/runners/_specific_runners.html.haml b/app/views/projects/runners/_specific_runners.html.haml index 6b8e6bd4fee..f8835454140 100644 --- a/app/views/projects/runners/_specific_runners.html.haml +++ b/app/views/projects/runners/_specific_runners.html.haml @@ -9,7 +9,7 @@ (checkout the #{link_to 'GitLab Runner section', 'https://about.gitlab.com/gitlab-ci/#gitlab-runner', target: '_blank'} for information on how to install it). %li Specify the following URL during the Runner setup: - %code= ci_root_url(only_path: false) + %code= root_url(only_path: false) %li Use the following registration token during setup: %code= @project.runners_token diff --git a/app/views/projects/tree/_readme.html.haml b/app/views/projects/tree/_readme.html.haml index 2c2f64283f5..de57cd4ba00 100644 --- a/app/views/projects/tree/_readme.html.haml +++ b/app/views/projects/tree/_readme.html.haml @@ -1,8 +1,9 @@ -%article.file-holder.readme-holder - .js-file-title.file-title - = blob_icon readme.mode, readme.name - = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@ref, readme.path)) do - %strong - = readme.name +- if readme.rich_viewer + %article.file-holder.readme-holder + .js-file-title.file-title + = blob_icon readme.mode, readme.name + = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@ref, readme.path)) do + %strong + = readme.name - = render 'projects/blob/viewer', viewer: readme.rich_viewer, viewer_url: namespace_project_blob_path(@project.namespace, @project, tree_join(@ref, readme.path), viewer: :rich, format: :json) + = render 'projects/blob/viewer', viewer: readme.rich_viewer, viewer_url: namespace_project_blob_path(@project.namespace, @project, tree_join(@ref, readme.path), viewer: :rich, format: :json) diff --git a/changelogs/unreleased/gitaly-local-branches.yml b/changelogs/unreleased/gitaly-local-branches.yml new file mode 100644 index 00000000000..adcc0fa6280 --- /dev/null +++ b/changelogs/unreleased/gitaly-local-branches.yml @@ -0,0 +1,4 @@ +--- +title: Add suport for find_local_branches GRPC from Gitaly +merge_request: 10059 +author: diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index ffa0831290a..408d46a756c 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -37,7 +37,7 @@ GitLab Runner then executes job scripts as the `gitlab-runner` user. ```bash sudo gitlab-ci-multi-runner register -n \ - --url https://gitlab.com/ci \ + --url https://gitlab.com/ \ --registration-token REGISTRATION_TOKEN \ --executor shell \ --description "My Runner" @@ -94,7 +94,7 @@ In order to do that, follow the steps: ```bash sudo gitlab-ci-multi-runner register -n \ - --url https://gitlab.com/ci \ + --url https://gitlab.com/ \ --registration-token REGISTRATION_TOKEN \ --executor docker \ --description "My Docker Runner" \ @@ -112,7 +112,7 @@ In order to do that, follow the steps: ``` [[runners]] - url = "https://gitlab.com/ci" + url = "https://gitlab.com/" token = TOKEN executor = "docker" [runners.docker] @@ -179,7 +179,7 @@ In order to do that, follow the steps: ```bash sudo gitlab-ci-multi-runner register -n \ - --url https://gitlab.com/ci \ + --url https://gitlab.com/ \ --registration-token REGISTRATION_TOKEN \ --executor docker \ --description "My Docker Runner" \ @@ -197,7 +197,7 @@ In order to do that, follow the steps: ``` [[runners]] - url = "https://gitlab.com/ci" + url = "https://gitlab.com/" token = REGISTRATION_TOKEN executor = "docker" [runners.docker] diff --git a/doc/ci/examples/test-and-deploy-python-application-to-heroku.md b/doc/ci/examples/test-and-deploy-python-application-to-heroku.md index e4d3970deac..73aebaf6d7f 100644 --- a/doc/ci/examples/test-and-deploy-python-application-to-heroku.md +++ b/doc/ci/examples/test-and-deploy-python-application-to-heroku.md @@ -55,11 +55,11 @@ You can do this through the [Dashboard](https://dashboard.heroku.com/). ### Create runner First install [Docker Engine](https://docs.docker.com/installation/). To build this project you also need to have [GitLab Runner](https://about.gitlab.com/gitlab-ci/#gitlab-runner). -You can use public runners available on `gitlab.com/ci`, but you can register your own: +You can use public runners available on `gitlab.com`, but you can register your own: ``` gitlab-ci-multi-runner register \ --non-interactive \ - --url "https://gitlab.com/ci/" \ + --url "https://gitlab.com/" \ --registration-token "PROJECT_REGISTRATION_TOKEN" \ --description "python-3.5" \ --executor "docker" \ diff --git a/doc/ci/examples/test-and-deploy-ruby-application-to-heroku.md b/doc/ci/examples/test-and-deploy-ruby-application-to-heroku.md index 42f15a27f12..6fa64a67e82 100644 --- a/doc/ci/examples/test-and-deploy-ruby-application-to-heroku.md +++ b/doc/ci/examples/test-and-deploy-ruby-application-to-heroku.md @@ -50,11 +50,11 @@ You can do this through the [Dashboard](https://dashboard.heroku.com/). ### Create runner First install [Docker Engine](https://docs.docker.com/installation/). To build this project you also need to have [GitLab Runner](https://about.gitlab.com/gitlab-ci/#gitlab-runner). -You can use public runners available on `gitlab.com/ci`, but you can register your own: +You can use public runners available on `gitlab.com`, but you can register your own: ``` gitlab-ci-multi-runner register \ --non-interactive \ - --url "https://gitlab.com/ci/" \ + --url "https://gitlab.com/" \ --registration-token "PROJECT_REGISTRATION_TOKEN" \ --description "ruby-2.2" \ --executor "docker" \ diff --git a/lib/gitlab/git/branch.rb b/lib/gitlab/git/branch.rb index 586380da94a..124526e4b59 100644 --- a/lib/gitlab/git/branch.rb +++ b/lib/gitlab/git/branch.rb @@ -1,6 +1,40 @@ module Gitlab module Git class Branch < Ref + def initialize(repository, name, target) + if target.is_a?(Gitaly::FindLocalBranchResponse) + target = target_from_gitaly_local_branches_response(target) + end + + super(repository, name, target) + end + + def target_from_gitaly_local_branches_response(response) + # Git messages have no encoding enforcements. However, in the UI we only + # handle UTF-8, so basically we cross our fingers that the message force + # encoded to UTF-8 is readable. + message = response.commit_subject.dup.force_encoding('UTF-8') + + # NOTE: For ease of parsing in Gitaly, we have only the subject of + # the commit and not the full message. This is ok, since all the + # code that uses `local_branches` only cares at most about the + # commit message. + # TODO: Once gitaly "takes over" Rugged consider separating the + # subject from the message to make it clearer when there's one + # available but not the other. + hash = { + id: response.commit_id, + message: message, + authored_date: Time.at(response.commit_author.date.seconds), + author_name: response.commit_author.name, + author_email: response.commit_author.email, + committed_date: Time.at(response.commit_committer.date.seconds), + committer_name: response.commit_committer.name, + committer_email: response.commit_committer.email + } + + Gitlab::Git::Commit.decorate(hash) + end end end end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index f9a9b767ef4..297531db4cc 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -19,13 +19,7 @@ module Gitlab def ==(other) return false unless other.is_a?(Gitlab::Git::Commit) - methods = [:message, :parent_ids, :authored_date, :author_name, - :author_email, :committed_date, :committer_name, - :committer_email] - - methods.all? do |method| - send(method) == other.send(method) - end + id && id == other.id end class << self @@ -55,6 +49,7 @@ module Gitlab # Commit.find(repo, 'master') # def find(repo, commit_id = "HEAD") + return commit_id if commit_id.is_a?(Gitlab::Git::Commit) return decorate(commit_id) if commit_id.is_a?(Rugged::Commit) obj = if commit_id.is_a?(String) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index a2c01ec4432..b9f1ac144b6 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -80,14 +80,16 @@ module Gitlab end # Returns an Array of Branches - def branches - rugged.branches.map do |rugged_ref| + def branches(filter: nil, sort_by: nil) + branches = rugged.branches.each(filter).map do |rugged_ref| begin Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target) rescue Rugged::ReferenceError # Omit invalid branch end - end.compact.sort_by(&:name) + end.compact + + sort_branches(branches, sort_by) end def reload_rugged @@ -108,9 +110,15 @@ module Gitlab Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target) if rugged_ref end - def local_branches - rugged.branches.each(:local).map do |branch| - Gitlab::Git::Branch.new(self, branch.name, branch.target) + def local_branches(sort_by: nil) + gitaly_migrate(:local_branches) do |is_enabled| + if is_enabled + gitaly_ref_client.local_branches(sort_by: sort_by).map do |gitaly_branch| + Gitlab::Git::Branch.new(self, gitaly_branch.name, gitaly_branch) + end + else + branches(filter: :local, sort_by: sort_by) + end end end @@ -1202,6 +1210,23 @@ module Gitlab diff.each_patch end + def sort_branches(branches, sort_by) + case sort_by + when 'name' + branches.sort_by(&:name) + when 'updated_desc' + branches.sort do |a, b| + b.dereferenced_target.committed_date <=> a.dereferenced_target.committed_date + end + when 'updated_asc' + branches.sort do |a, b| + a.dereferenced_target.committed_date <=> b.dereferenced_target.committed_date + end + else + branches + end + end + def gitaly_ref_client @gitaly_ref_client ||= Gitlab::GitalyClient::Ref.new(self) end diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb index 53c43e28df8..227fe45642e 100644 --- a/lib/gitlab/gitaly_client/ref.rb +++ b/lib/gitlab/gitaly_client/ref.rb @@ -44,6 +44,12 @@ module Gitlab branch_names.count end + def local_branches(sort_by: nil) + request = Gitaly::FindLocalBranchesRequest.new(repository: @gitaly_repo) + request.sort_by = sort_by_param(sort_by) if sort_by + consume_branches_response(stub.find_local_branches(request)) + end + private def consume_refs_response(response, prefix:) @@ -51,6 +57,16 @@ module Gitlab r.names.map { |name| name.sub(/\A#{Regexp.escape(prefix)}/, '') } end end + + def sort_by_param(sort_by) + enum_value = Gitaly::FindLocalBranchesRequest::SortBy.resolve(sort_by.upcase.to_sym) + raise ArgumentError, "Invalid sort_by key `#{sort_by}`" unless enum_value + enum_value + end + + def consume_branches_response(response) + response.flat_map { |r| r.branches } + end end end end diff --git a/spec/factories/group_members.rb b/spec/factories/group_members.rb index 080b2e75ea1..32cbfe28a60 100644 --- a/spec/factories/group_members.rb +++ b/spec/factories/group_members.rb @@ -10,5 +10,11 @@ FactoryGirl.define do trait(:master) { access_level GroupMember::MASTER } trait(:owner) { access_level GroupMember::OWNER } trait(:access_request) { requested_at Time.now } + + trait(:invited) do + user_id nil + invite_token 'xxx' + invite_email 'email@email.com' + end end end diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb index d62799a5a47..fe4518caadf 100644 --- a/spec/factories/project_members.rb +++ b/spec/factories/project_members.rb @@ -9,5 +9,11 @@ FactoryGirl.define do trait(:developer) { access_level ProjectMember::DEVELOPER } trait(:master) { access_level ProjectMember::MASTER } trait(:access_request) { requested_at Time.now } + + trait(:invited) do + user_id nil + invite_token 'xxx' + invite_email 'email@email.com' + end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 3580752a805..7a76f5f8afc 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -60,7 +60,9 @@ FactoryGirl.define do trait :test_repo do after :create do |project| - TestEnv.copy_repo(project) + TestEnv.copy_repo(project, + bare_repo: TestEnv.factory_repo_path_bare, + refs: TestEnv::BRANCH_SHA) end end @@ -139,7 +141,9 @@ FactoryGirl.define do end after :create do |project, evaluator| - TestEnv.copy_repo(project) + TestEnv.copy_repo(project, + bare_repo: TestEnv.factory_repo_path_bare, + refs: TestEnv::BRANCH_SHA) if evaluator.create_template args = evaluator.create_template @@ -172,7 +176,9 @@ FactoryGirl.define do path { 'forked-gitlabhq' } after :create do |project| - TestEnv.copy_forked_repo_with_submodules(project) + TestEnv.copy_repo(project, + bare_repo: TestEnv.forked_repo_path_bare, + refs: TestEnv::FORKED_BRANCH_SHA) end end diff --git a/spec/features/merge_requests/diffs_spec.rb b/spec/features/merge_requests/diffs_spec.rb index 7dee3b852ca..4860a2a7498 100644 --- a/spec/features/merge_requests/diffs_spec.rb +++ b/spec/features/merge_requests/diffs_spec.rb @@ -20,6 +20,34 @@ feature 'Diffs URL', js: true, feature: true do end end + context 'when linking to note' do + describe 'with unresolved note' do + let(:note) { create :diff_note_on_merge_request, project: project, noteable: merge_request } + let(:fragment) { "#note_#{note.id}" } + + before do + visit "#{diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)}#{fragment}" + end + + it 'shows expanded note' do + expect(page).to have_selector(fragment, visible: true) + end + end + + describe 'with resolved note' do + let(:note) { create :diff_note_on_merge_request, :resolved, project: project, noteable: merge_request } + let(:fragment) { "#note_#{note.id}" } + + before do + visit "#{diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)}#{fragment}" + end + + it 'shows expanded note' do + expect(page).to have_selector(fragment, visible: true) + end + end + end + context 'when merge request has overflow' do it 'displays warning' do allow(Commit).to receive(:max_diff_options).and_return(max_files: 3) diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index fc9b293c393..884d1bbb10c 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -Dir["./spec/features/protected_branches/*.rb"].sort.each { |f| require f } feature 'Projected Branches', feature: true, js: true do let(:user) { create(:user, :admin) } diff --git a/spec/features/protected_tags_spec.rb b/spec/features/protected_tags_spec.rb index e68448467b0..66236dbc7fc 100644 --- a/spec/features/protected_tags_spec.rb +++ b/spec/features/protected_tags_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -Dir["./spec/features/protected_tags/*.rb"].sort.each { |f| require f } feature 'Projected Tags', feature: true, js: true do let(:user) { create(:user, :admin) } diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index cdf1b8beee3..9eac7660cd1 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -7,6 +7,51 @@ describe Gitlab::Git::Branch, seed_helper: true do it { is_expected.to be_kind_of Array } + describe 'initialize' do + let(:commit_id) { 'f00' } + let(:commit_subject) { "My commit".force_encoding('ASCII-8BIT') } + let(:committer) do + Gitaly::FindLocalBranchCommitAuthor.new( + name: generate(:name), + email: generate(:email), + date: Google::Protobuf::Timestamp.new(seconds: 123) + ) + end + let(:author) do + Gitaly::FindLocalBranchCommitAuthor.new( + name: generate(:name), + email: generate(:email), + date: Google::Protobuf::Timestamp.new(seconds: 456) + ) + end + let(:gitaly_branch) do + Gitaly::FindLocalBranchResponse.new( + name: 'foo', commit_id: commit_id, commit_subject: commit_subject, + commit_author: author, commit_committer: committer + ) + end + let(:attributes) do + { + id: commit_id, + message: commit_subject, + authored_date: Time.at(author.date.seconds), + author_name: author.name, + author_email: author.email, + committed_date: Time.at(committer.date.seconds), + committer_name: committer.name, + committer_email: committer.email + } + end + let(:branch) { described_class.new(repository, 'foo', gitaly_branch) } + + it 'parses Gitaly::FindLocalBranchResponse correctly' do + expect(Gitlab::Git::Commit).to receive(:decorate). + with(hash_including(attributes)).and_call_original + + expect(branch.dereferenced_target.message.encoding).to be(Encoding::UTF_8) + end + end + describe '#size' do subject { super().size } it { is_expected.to eq(SeedRepo::Repo::BRANCHES.size) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 53d492b8f74..cb107c6d1f9 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1105,7 +1105,9 @@ describe Gitlab::Git::Repository, seed_helper: true do ref = double() allow(ref).to receive(:name) { 'bad-branch' } allow(ref).to receive(:target) { raise Rugged::ReferenceError } - allow(repository.rugged).to receive(:branches) { [ref] } + branches = double() + allow(branches).to receive(:each) { [ref].each } + allow(repository.rugged).to receive(:branches) { branches } end it 'should return empty branches' do @@ -1289,7 +1291,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#local_branches' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git')) end after(:all) do @@ -1304,6 +1306,29 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(@repo.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) expect(@repo.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true) end + + context 'with gitaly enabled' do + before { stub_gitaly } + after { Gitlab::GitalyClient.clear_stubs! } + + it 'gets the branches from GitalyClient' do + expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches). + and_return([]) + @repo.local_branches + end + + it 'wraps GRPC not found' do + expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches). + and_raise(GRPC::NotFound) + expect { @repo.local_branches }.to raise_error(Gitlab::Git::Repository::NoRepository) + end + + it 'wraps GRPC exceptions' do + expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches). + and_raise(GRPC::Unknown) + expect { @repo.local_branches }.to raise_error(Gitlab::Git::CommandError) + end + end end def create_remote_branch(remote_name, branch_name, source_branch_name) diff --git a/spec/lib/gitlab/gitaly_client/ref_spec.rb b/spec/lib/gitlab/gitaly_client/ref_spec.rb index 255f23e6270..d8cd2dcbd2a 100644 --- a/spec/lib/gitlab/gitaly_client/ref_spec.rb +++ b/spec/lib/gitlab/gitaly_client/ref_spec.rb @@ -9,6 +9,13 @@ describe Gitlab::GitalyClient::Ref do allow(Gitlab.config.gitaly).to receive(:enabled).and_return(true) end + after do + # When we say `expect_any_instance_of(Gitaly::Ref::Stub)` a double is created, + # and because GitalyClient shares stubs these will get passed from example to + # example, which will cause an error, so we clean the stubs after each example. + Gitlab::GitalyClient.clear_stubs! + end + describe '#branch_names' do it 'sends a find_all_branch_names message' do expect_any_instance_of(Gitaly::Ref::Stub). @@ -38,4 +45,27 @@ describe Gitlab::GitalyClient::Ref do client.default_branch_name end end + + describe '#local_branches' do + it 'sends a find_local_branches message' do + expect_any_instance_of(Gitaly::Ref::Stub). + to receive(:find_local_branches).with(gitaly_request_with_repo_path(repo_path)). + and_return([]) + + client.local_branches + end + + it 'parses and sends the sort parameter' do + expect_any_instance_of(Gitaly::Ref::Stub). + to receive(:find_local_branches). + with(gitaly_request_with_params(sort_by: :UPDATED_DESC)). + and_return([]) + + client.local_branches(sort_by: 'updated_desc') + end + + it 'raises an argument error if an invalid sort_by parameter is passed' do + expect { client.local_branches(sort_by: 'invalid_sort') }.to raise_error(ArgumentError) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e6e7774431e..6a15830a15c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -929,10 +929,20 @@ describe User, models: true do end context 'with a group route matching the given path' do - let!(:group) { create(:group, path: 'group_path') } + context 'when the group namespace has an owner_id (legacy data)' do + let!(:group) { create(:group, path: 'group_path', owner: user) } - it 'returns nil' do - expect(User.find_by_full_path('group_path')).to eq(nil) + it 'returns nil' do + expect(User.find_by_full_path('group_path')).to eq(nil) + end + end + + context 'when the group namespace does not have an owner_id' do + let!(:group) { create(:group, path: 'group_path') } + + it 'returns nil' do + expect(User.find_by_full_path('group_path')).to eq(nil) + end end end end diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb index ab440d18e9f..8a6732faa19 100644 --- a/spec/services/members/authorized_destroy_service_spec.rb +++ b/spec/services/members/authorized_destroy_service_spec.rb @@ -10,6 +10,27 @@ describe Members::AuthorizedDestroyService, services: true do Issue.assigned_to(user).count + MergeRequest.assigned_to(user).count end + context 'Invited users' do + # Regression spec for issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/32504 + it 'destroys invited project member' do + project.team << [member_user, :developer] + + member = create :project_member, :invited, project: project + + expect { described_class.new(member, member_user).execute } + .to change { Member.count }.from(2).to(1) + end + + it 'destroys invited group member' do + group.add_developer(member_user) + + member = create :group_member, :invited, group: group + + expect { described_class.new(member, member_user).execute } + .to change { Member.count }.from(2).to(1) + end + end + context 'Group member' do it "unassigns issues and merge requests" do group.add_developer(member_user) diff --git a/spec/support/matchers/gitaly_matchers.rb b/spec/support/matchers/gitaly_matchers.rb index 65dbc01f6e4..ed14bcec9f2 100644 --- a/spec/support/matchers/gitaly_matchers.rb +++ b/spec/support/matchers/gitaly_matchers.rb @@ -1,3 +1,9 @@ RSpec::Matchers.define :gitaly_request_with_repo_path do |path| match { |actual| actual.repository.path == path } end + +RSpec::Matchers.define :gitaly_request_with_params do |params| + match do |actual| + params.reduce(true) { |r, (key, val)| r && actual.send(key) == val } + end +end diff --git a/spec/features/protected_branches/access_control_ce_spec.rb b/spec/support/protected_branches/access_control_ce_shared_examples.rb index 7fda4ade665..7fda4ade665 100644 --- a/spec/features/protected_branches/access_control_ce_spec.rb +++ b/spec/support/protected_branches/access_control_ce_shared_examples.rb diff --git a/spec/features/protected_tags/access_control_ce_spec.rb b/spec/support/protected_tags/access_control_ce_shared_examples.rb index 12622cd548a..12622cd548a 100644 --- a/spec/features/protected_tags/access_control_ce_spec.rb +++ b/spec/support/protected_tags/access_control_ce_shared_examples.rb diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 9bf9dc5d4b2..b168098edea 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -40,8 +40,8 @@ module TestEnv 'wip' => 'b9238ee', 'csv' => '3dd0896', 'v1.1.0' => 'b83d6e3', - 'add-ipython-files' => '6d85bb69', - 'add-pdf-file' => 'e774ebd3' + 'add-ipython-files' => '6d85bb6', + 'add-pdf-file' => 'e774ebd' }.freeze # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily @@ -155,14 +155,14 @@ module TestEnv FORKED_BRANCH_SHA) end - def setup_repo(repo_path, repo_path_bare, repo_name, branch_sha) + def setup_repo(repo_path, repo_path_bare, repo_name, refs) clone_url = "https://gitlab.com/gitlab-org/#{repo_name}.git" unless File.directory?(repo_path) system(*%W(#{Gitlab.config.git.bin_path} clone -q #{clone_url} #{repo_path})) end - set_repo_refs(repo_path, branch_sha) + set_repo_refs(repo_path, refs) unless File.directory?(repo_path_bare) # We must copy bare repositories because we will push to them. @@ -170,13 +170,12 @@ module TestEnv end end - def copy_repo(project) - base_repo_path = File.expand_path(factory_repo_path_bare) + def copy_repo(project, bare_repo:, refs:) target_repo_path = File.expand_path(project.repository_storage_path + "/#{project.full_path}.git") FileUtils.mkdir_p(target_repo_path) - FileUtils.cp_r("#{base_repo_path}/.", target_repo_path) + FileUtils.cp_r("#{File.expand_path(bare_repo)}/.", target_repo_path) FileUtils.chmod_R 0755, target_repo_path - set_repo_refs(target_repo_path, BRANCH_SHA) + set_repo_refs(target_repo_path, refs) end def repos_path @@ -191,15 +190,6 @@ module TestEnv Gitlab.config.pages.path end - def copy_forked_repo_with_submodules(project) - base_repo_path = File.expand_path(forked_repo_path_bare) - target_repo_path = File.expand_path(project.repository_storage_path + "/#{project.full_path}.git") - FileUtils.mkdir_p(target_repo_path) - FileUtils.cp_r("#{base_repo_path}/.", target_repo_path) - FileUtils.chmod_R 0755, target_repo_path - set_repo_refs(target_repo_path, FORKED_BRANCH_SHA) - end - # When no cached assets exist, manually hit the root path to create them # # Otherwise they'd be created by the first test, often timing out and @@ -211,16 +201,20 @@ module TestEnv Capybara.current_session.visit '/' end + def factory_repo_path_bare + "#{factory_repo_path}_bare" + end + + def forked_repo_path_bare + "#{forked_repo_path}_bare" + end + private def factory_repo_path @factory_repo_path ||= Rails.root.join('tmp', 'tests', factory_repo_name) end - def factory_repo_path_bare - "#{factory_repo_path}_bare" - end - def factory_repo_name 'gitlab-test' end @@ -229,10 +223,6 @@ module TestEnv @forked_repo_path ||= Rails.root.join('tmp', 'tests', forked_repo_name) end - def forked_repo_path_bare - "#{forked_repo_path}_bare" - end - def forked_repo_name 'gitlab-test-fork' end @@ -244,19 +234,22 @@ module TestEnv end def set_repo_refs(repo_path, branch_sha) - instructions = branch_sha.map {|branch, sha| "update refs/heads/#{branch}\x00#{sha}\x00" }.join("\x00") << "\x00" + instructions = branch_sha.map { |branch, sha| "update refs/heads/#{branch}\x00#{sha}\x00" }.join("\x00") << "\x00" update_refs = %W(#{Gitlab.config.git.bin_path} update-ref --stdin -z) reset = proc do - IO.popen(update_refs, "w") {|io| io.write(instructions) } - $?.success? + Dir.chdir(repo_path) do + IO.popen(update_refs, "w") { |io| io.write(instructions) } + $?.success? + end end - Dir.chdir(repo_path) do - # Try to reset without fetching to avoid using the network. - unless reset.call - raise 'Could not fetch test seed repository.' unless system(*%W(#{Gitlab.config.git.bin_path} fetch origin)) - raise 'The fetched test seed does not contain the required revision.' unless reset.call - end + # Try to reset without fetching to avoid using the network. + unless reset.call + raise 'Could not fetch test seed repository.' unless system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} fetch origin)) + + # Before we used Git clone's --mirror option, bare repos could end up + # with missing refs, clearing them and retrying should fix the issue. + cleanup && init unless reset.call end end end |