Commit 7efc5eba authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'ce-to-ee-2018-07-17' into 'master'

CE upstream - 2018-07-17 12:23 UTC

Closes gitaly#748, gitaly#695 et gitaly#799

See merge request gitlab-org/gitlab-ee!6534
parents b1c08506 a0d034b8
......@@ -34,17 +34,17 @@ When removing columns, tables, indexes or other structures:
## General checklist
- [ ] [Changelog entry](https://docs.gitlab.com/ee/development/changelog.html) added, if necessary
- [ ] [Documentation created/updated](https://docs.gitlab.com/ee/development/doc_styleguide.html)
- [ ] API support added
- [ ] Tests added for this feature/bug
- Conform by the [code review guidelines](https://docs.gitlab.com/ee/development/code_review.html)
- [ ] Has been reviewed by a Backend maintainer
- [ ] Has been reviewed by a Database specialist
- [ ] Conform by the [merge request performance guides](https://docs.gitlab.com/ee/development/merge_request_performance_guidelines.html)
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/CONTRIBUTING.md#style-guides)
- [ ] [Documentation created/updated](https://docs.gitlab.com/ee/development/documentation/index.html#contributing-to-docs)
- [ ] [API support added](https://docs.gitlab.com/ee/development/api_styleguide.html)
- [ ] [Tests added for this feature/bug](https://docs.gitlab.com/ee/development/testing_guide/index.html)
- Conforms to the [code review guidelines](https://docs.gitlab.com/ee/development/code_review.html)
- [ ] Has been reviewed by a Backend [maintainer](https://about.gitlab.com/handbook/engineering/#maintainer)
- [ ] Has been reviewed by a Database [specialist](https://about.gitlab.com/team/structure/#specialist)
- [ ] Conforms to the [merge request performance guidelines](https://docs.gitlab.com/ee/development/merge_request_performance_guidelines.html)
- [ ] Conforms to the [style guides](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/CONTRIBUTING.md#style-guides)
- [ ] If you have multiple commits, please combine them into a few logically organized commits by [squashing them](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
- [ ] Internationalization required/considered
- [ ] If paid feature, have we considered GitLab.com plan and how it works for groups and is there a design for promoting it to users who aren't on the correct plan
- [ ] End-to-end tests pass (`package-and-qa` manual pipeline job)
- [ ] [Internationalization required/considered](https://docs.gitlab.com/ee/development/i18n/index.html)
- [ ] For a paid feature, have we considered GitLab.com plans, how it works for groups, and is there a design for promoting it to users who aren't on the correct plan?
- [ ] [End-to-end tests](https://docs.gitlab.com/ee/development/testing_guide/end_to_end_tests.html#testing-code-in-merge-requests) pass (`package-and-qa` manual pipeline job)
/label ~database
<!--See the general Documentation guidelines https://docs.gitlab.com/ce/development/writing_documentation.html -->
<!--See the general Documentation guidelines https://docs.gitlab.com/ee/development/documentation/index.html -->
## What does this MR do?
......@@ -13,17 +13,17 @@ Closes
## Moving docs to a new location?
Read the guidelines:
https://docs.gitlab.com/ce/development/writing_documentation.html#changing-document-location
https://docs.gitlab.com/ee/development/documentation/#changing-document-location
- [ ] Make sure the old link is not removed and has its contents replaced with
a link to the new location.
- [ ] Make sure internal links pointing to the document in question are not broken.
- [ ] Search and replace any links referring to old docs in GitLab Rails app,
- [ ] Search and replace any links referring to the old docs in the GitLab Rails app,
specifically under the `app/views/` and `ee/app/views` (for GitLab EE) directories.
- [ ] Make sure to add [`redirect_from`](https://docs.gitlab.com/ce/development/writing_documentation.html#redirections-for-pages-with-disqus-comments)
- [ ] Make sure to add [`redirect_from`](https://docs.gitlab.com/ee/development/documentation/index.html#redirections-for-pages-with-disqus-comments)
to the new document if there are any Disqus comments on the old document thread.
- [ ] If working on CE and the `ee-compat-check` jobs fails, submit an MR to EE
with the changes as well (https://docs.gitlab.com/ce/development/writing_documentation.html#cherry-picking-from-ce-to-ee).
- [ ] If working on CE and the `ee-compat-check` jobs fails, [submit an MR to EE
with the changes](https://docs.gitlab.com/ee/development/documentation/index.html#cherry-picking-from-ce-to-ee) as well.
- [ ] Ping one of the technical writers for review.
/label ~Documentation
......@@ -116,7 +116,12 @@ class Projects::WikisController < Projects::ApplicationController
# Call #wiki to make sure the Wiki Repo is initialized
@project_wiki.wiki
@sidebar_page = @project_wiki.find_sidebar(params[:version_id])
unless @sidebar_page # Fallback to default sidebar
@sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.pages(limit: 15))
end
rescue ProjectWiki::CouldNotCreateWikiError
flash[:notice] = "Could not create Wiki Repository at this time. Please try again later."
redirect_to project_path(@project)
......
......@@ -2185,12 +2185,15 @@ class Project < ActiveRecord::Base
merge_requests = source_of_merge_requests.opened
.where(allow_collaboration: true)
# Issue for N+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/49322
Gitlab::GitalyClient.allow_n_plus_1_calls do
if branch_name
merge_requests.find_by(source_branch: branch_name)&.can_be_merged_by?(user)
else
merge_requests.any? { |merge_request| merge_request.can_be_merged_by?(user) }
end
end
end
if RequestStore.active?
RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_collaboration") do
......
# frozen_string_literal: true
class ProjectWiki
include Gitlab::ShellAdapter
include Storage::LegacyProjectWiki
......@@ -12,6 +14,7 @@ class ProjectWiki
}.freeze unless defined?(MARKUPS)
CouldNotCreateWikiError = Class.new(StandardError)
SIDEBAR = '_sidebar'
# Returns a string describing what went wrong after
# an operation fails.
......@@ -106,6 +109,10 @@ class ProjectWiki
end
end
def find_sidebar(version = nil)
find_page(SIDEBAR, version)
end
def find_file(name, version = nil)
wiki.file(name, version)
end
......
......@@ -11,7 +11,7 @@
- branch_label = s_('ChangeTypeActionLabel|Pick into branch')
- title = commit.merged_merge_request(current_user) ? _('Cherry-pick this merge request') : _('Cherry-pick this commit')
.modal{ id: "modal-#{type}-commit" }
.modal{ id: "modal-#{type}-commit", tabindex: -1 }
.modal-dialog
.modal-content
.modal-header
......
......@@ -11,9 +11,11 @@
.blocks-container
.block.block-first
- if @sidebar_page
= render_wiki_content(@sidebar_page)
- else
%ul.wiki-pages
= render @sidebar_wiki_entries, context: 'sidebar'
.block
= link_to project_wikis_pages_path(@project), class: 'btn btn-block' do
= s_("Wiki|More Pages")
......
---
title: "Custom Wiki Sidebar Support Issue 14995"
merge_request:
author: Josh Sooter
type: added
---
title: Close revert and cherry pick modal on escape keypress
merge_request: 20341
author: George Tsiolis
type: changed
---
title: Add emails delivery Prometheus metrics
merge_request: 20638
author:
type: added
unless Gitlab.config.gitlab.email_enabled
ActionMailer::Base.register_interceptor(::Gitlab::Email::Hook::DisableEmailInterceptor)
ActionMailer::Base.logger = nil
end
ActionMailer::Base.register_interceptors(
::Gitlab::Email::Hook::AdditionalHeadersInterceptor,
::Gitlab::Email::Hook::EmailTemplateInterceptor,
::Gitlab::Email::Hook::DeliveryMetricsObserver
)
ActionMailer::Base.register_observer(::Gitlab::Email::Hook::DeliveryMetricsObserver)
ActionMailer::Base.register_interceptor(AdditionalEmailHeadersInterceptor)
# Interceptor in lib/disable_email_interceptor.rb
unless Gitlab.config.gitlab.email_enabled
ActionMailer::Base.register_interceptor(DisableEmailInterceptor)
ActionMailer::Base.logger = nil
end
# Interceptor in lib/email_template_interceptor.rb
ActionMailer::Base.register_interceptor(EmailTemplateInterceptor)
......@@ -243,3 +243,45 @@ WHERE EXISTS (
```
[gin-index]: http://www.postgresql.org/docs/current/static/gin.html
## `.find_or_create_by` is not atomic
The inherent pattern with methods like `.find_or_create_by` and
`.first_or_create` and others is that they are not atomic. This means,
it first runs a `SELECT`, and if there are no results an `INSERT` is
performed. With concurrent processes in mind, there is a race condition
which may lead to trying to insert two similar records. This may not be
desired, or may cause one of the queries to fail due to a constraint
violation, for example.
Using transactions does not solve this problem.
The following pattern should be used to avoid the problem:
```ruby
Project.transaction do
begin
User.find_or_create_by(username: "foo")
rescue ActiveRecord::RecordNotUnique
retry
end
end
```
If the above block is run inside a transaction and hits the race
condition, the transaction is aborted and we cannot simply retry (any
further queries inside the aborted transaction are going to fail). We
can employ [nested transactions](http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Nested+transactions)
here to only rollback the "inner transaction". Note that `requires_new: true` is required here.
```ruby
Project.transaction do
begin
User.transaction(requires_new: true) do
User.find_or_create_by(username: "foo")
end
rescue ActiveRecord::RecordNotUnique
retry
end
end
```
# Import multiple repositories by uploading a manifest file
GitLab allows you to import all the required git repositories
based a manifest file like the one used by the Android repository.
based a manifest file like the one used by the [Android repository](https://android.googlesource.com/platform/manifest/+/2d6f081a3b05d8ef7a2b1b52b0d536b2b74feab4/default.xml).
This feature can be very handy when you need to import a project with many repositories like Android Open Source Project (AOSP).
>**Note:**
......
......@@ -107,3 +107,10 @@ On the right sidebar, click on **Clone repository** and follow the on-screen
instructions.
[permissions]: ../../permissions.md
## Customizing sidebar
By default, the wiki would render a sidebar which lists all the pages for the
wiki. You could as well provide a `_sidebar` page to replace this default
sidebar. When this customized sidebar page is provided, the default sidebar
would not be rendered, but the customized one.
class AdditionalEmailHeadersInterceptor
def self.delivering_email(message)
message.header['Auto-Submitted'] ||= 'auto-generated'
message.header['X-Auto-Response-Suppress'] ||= 'All'
end
end
# Read about interceptors in http://guides.rubyonrails.org/action_mailer_basics.html#intercepting-emails
class DisableEmailInterceptor
def self.delivering_email(message)
message.perform_deliveries = false
Rails.logger.info "Emails disabled! Interceptor prevented sending mail #{message.subject}"
end
end
# Read about interceptors in http://guides.rubyonrails.org/action_mailer_basics.html#intercepting-emails
class EmailTemplateInterceptor
def self.delivering_email(message)
# Remove HTML part if HTML emails are disabled.
unless Gitlab::CurrentSettings.html_emails_enabled
message.parts.delete_if do |part|
part.content_type.start_with?('text/html')
end
end
end
end
module Gitlab
module Email
module Hook
class AdditionalHeadersInterceptor
def self.delivering_email(message)
message.header['Auto-Submitted'] ||= 'auto-generated'
message.header['X-Auto-Response-Suppress'] ||= 'All'
end
end
end
end
end
module Gitlab
module Email
module Hook
class DeliveryMetricsObserver
extend Gitlab::Utils::StrongMemoize
def self.delivering_email(_message)
delivery_attempts_counter.increment
end
def self.delivered_email(_message)
delivered_emails_counter.increment
end
def self.delivery_attempts_counter
strong_memoize(:delivery_attempts_counter) do
Gitlab::Metrics.counter(:gitlab_emails_delivery_attempts_total,
'Counter of total emails delivery attempts')
end
end
def self.delivered_emails_counter
strong_memoize(:delivered_emails_counter) do
Gitlab::Metrics.counter(:gitlab_emails_delivered_total,
'Counter of total emails delievered')
end
end
end
end
end
end
module Gitlab
module Email
module Hook
class DisableEmailInterceptor
def self.delivering_email(message)
message.perform_deliveries = false
Rails.logger.info "Emails disabled! Interceptor prevented sending mail #{message.subject}"
end
end
end
end
end
module Gitlab
module Email
module Hook
class EmailTemplateInterceptor
##
# Remove HTML part if HTML emails are disabled.
#
def self.delivering_email(message)
unless Gitlab::CurrentSettings.html_emails_enabled
message.parts.delete_if do |part|
part.content_type.start_with?('text/html')
end
end
end
end
end
end
end
......@@ -443,12 +443,8 @@ module Gitlab
# Returns the SHA of the most recent common ancestor of +from+ and +to+
def merge_base(from, to)
gitaly_migrate(:merge_base) do |is_enabled|
if is_enabled
wrapped_gitaly_errors do
gitaly_repository_client.find_merge_base(from, to)
else
rugged_merge_base(from, to)
end
end
end
......@@ -464,12 +460,8 @@ module Gitlab
return [] unless root_sha
branches = gitaly_migrate(:merged_branch_names) do |is_enabled|
if is_enabled
branches = wrapped_gitaly_errors do
gitaly_merged_branch_names(branch_names, root_sha)
else
git_merged_branch_names(branch_names, root_sha)
end
end
Set.new(branches)
......@@ -848,12 +840,8 @@ module Gitlab
def write_ref(ref_path, ref, old_ref: nil, shell: true)
ref_path = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{ref_path}" unless ref_path.start_with?("refs/") || ref_path == "HEAD"
gitaly_migrate(:write_ref) do |is_enabled|
if is_enabled
wrapped_gitaly_errors do
gitaly_repository_client.write_ref(ref_path, ref, old_ref, shell)
else
local_write_ref(ref_path, ref, old_ref: old_ref, shell: shell)
end
end
end
......@@ -1188,37 +1176,6 @@ module Gitlab
end
end
def local_write_ref(ref_path, ref, old_ref: nil, shell: true)
if shell
shell_write_ref(ref_path, ref, old_ref)
else
rugged_write_ref(ref_path, ref)
end
end
def rugged_write_config(full_path:)
rugged.config['gitlab.fullpath'] = full_path
end
def shell_write_ref(ref_path, ref, old_ref)
raise ArgumentError, "invalid ref_path #{ref_path.inspect}" if ref_path.include?(' ')
raise ArgumentError, "invalid ref #{ref.inspect}" if ref.include?("\x00")
raise ArgumentError, "invalid old_ref #{old_ref.inspect}" if !old_ref.nil? && old_ref.include?("\x00")
input = "update #{ref_path}\x00#{ref}\x00#{old_ref}\x00"
run_git!(%w[update-ref --stdin -z]) { |stdin| stdin.write(input) }
end
def rugged_write_ref(ref_path, ref)
rugged.references.create(ref_path, ref, force: true)
rescue Rugged::ReferenceError => ex
Rails.logger.error "Unable to create #{ref_path} reference for repository #{path}: #{ex}"
rescue Rugged::OSError => ex
raise unless ex.message =~ /Failed to create locked file/ && ex.message =~ /File exists/
Rails.logger.error "Unable to create #{ref_path} reference for repository #{path}: #{ex}"
end
def run_git(args, chdir: path, env: {}, nice: false, lazy_block: nil, &block)
cmd = [Gitlab.config.git.bin_path, *args]
cmd.unshift("nice") if nice
......@@ -1289,20 +1246,6 @@ module Gitlab
}
end
def git_merged_branch_names(branch_names, root_sha)
git_arguments =
%W[branch --merged #{root_sha}
--format=%(refname:short)\ %(objectname)] + branch_names
lines = run_git(git_arguments).first.lines
lines.each_with_object([]) do |line, branches|
name, sha = line.strip.split(' ', 2)
branches << name if sha != root_sha
end
end
def gitaly_merged_branch_names(branch_names, root_sha)
qualified_branch_names = branch_names.map { |b| "refs/heads/#{b}" }
......@@ -1448,12 +1391,6 @@ module Gitlab
raise CommandError, @gitlab_projects.output
end
def rugged_merge_base(from, to)
rugged.merge_base(from, to)
rescue Rugged::ReferenceError
nil
end
def rev_list_param(spec)
spec == :all ? ['--all'] : spec
end
......
require 'rails_helper'
describe 'Merge request > User sees Check out branch modal', :js do
describe 'Merge request > User sees check out branch modal', :js do
let(:project) { create(:project, :public, :repository) }
let(:user) { project.creator }
let(:merge_request) { create(:merge_request, source_project: project) }
......@@ -16,7 +16,7 @@ describe 'Merge request > User sees Check out branch modal', :js do
expect(page).to have_content('Check out, review, and merge locally')
end
it 'closes the check out branch model with Escape keypress' do
it 'closes the check out branch modal with escape keypress' do
find('#modal_merge_info').send_keys(:escape)
expect(page).not_to have_content('Check out, review, and merge locally')
......
......@@ -21,7 +21,7 @@ describe 'Merge request > User cherry-picks', :js do
end
# Fast-forward merge, or merged before GitLab 8.5.
context 'Without a merge commit' do
context 'without a merge commit' do
before do
merge_request.merge_commit_sha = nil
merge_request.save
......@@ -34,7 +34,7 @@ describe 'Merge request > User cherry-picks', :js do
end
end
context 'With a merge commit' do
context 'with a merge commit' do
it 'shows a Cherry-pick button' do
visit project_merge_request_path(project, merge_request)
......@@ -49,5 +49,23 @@ describe 'Merge request > User cherry-picks', :js do
expect(page).not_to have_link 'Cherry-pick'
end
end
context 'and seeing the cherry-pick modal' do
before do
visit project_merge_request_path(project, merge_request)
click_link('Cherry-pick')
end
it 'shows the cherry-pick modal' do
expect(page).to have_content('Cherry-pick this merge request')
end
it 'closes the cherry-pick modal with escape keypress' do
find('#modal-cherry-pick-commit').send_keys(:escape)
expect(page).not_to have_content('Start a new merge request with these changes')
end
end
end
end
......@@ -2,16 +2,22 @@ require "spec_helper"
describe "User creates wiki page" do
let(:user) { create(:user) }
let(:wiki) { ProjectWiki.new(project, user) }
let(:project) { create(:project) }
before do
project.add_maintainer(user)
sign_in(user)
end
context "when wiki is empty" do
before do
visit(project_wikis_path(project))
click_link "Create your first page"
end
context "when wiki is empty" do
context "in a user namespace" do
let(:project) { create(:project, :wiki_repo, namespace: user.namespace) }
......@@ -165,7 +171,9 @@ describe "User creates wiki page" do
context "when wiki is not empty", :js do
before do
create(:wiki_page, wiki: create(:project, :wiki_repo, namespace: user.namespace).wiki, attrs: { title: "home", content: "Home page" })
create(:wiki_page, wiki: wiki, attrs: { title: 'home', content: 'Home page' })
visit(project_wikis_path(project))
end
context "in a user namespace" do
......@@ -290,4 +298,34 @@ describe "User creates wiki page" do
end
end
end
describe 'sidebar feature' do
context 'when there are some existing pages' do
before do
create(:wiki_page, wiki: wiki, attrs: { title: 'home', content: 'home' })
create(:wiki_page, wiki: wiki, attrs: { title: 'another', content: 'another' })
end
it 'renders a default sidebar when there is no customized sidebar' do
visit(project_wikis_path(project))
expect(page).to have_content('Another')
expect(page).to have_content('More Pages')
end
context 'when there is a customized sidebar' do
before do
create(:wiki_page, wiki: wiki, attrs: { title: '_sidebar', content: 'My customized sidebar' })
end
it 'renders my customized sidebar instead of the default one' do
visit(project_wikis_path(project))
expect(page).to have_content('My customized sidebar')
expect(page).to have_content('More Pages')
expect(page).not_to have_content('Another')
end
end
end
end
end
require 'rails_helper'
describe 'Merge request > User sees revert modal', :js do
let(:project) { create(:project, :public, :repository) }
let(:user) { project.creator }
let(:merge_request) { create(:merge_request, source_project: project) }
before do
sign_in(user)
visit(project_merge_request_path(project, merge_request))
click_button('Merge')
visit(merge_request_path(merge_request))
click_link('Revert')
end
it 'shows the revert modal' do
expect(page).to have_content('Revert this merge request')
end
it 'closes the revert modal with escape keypress' do
find('#modal-revert-commit').send_keys(:escape)
expect(page).not_to have_content('Revert this merge request')
end
end
require 'spec_helper'
describe AdditionalEmailHeadersInterceptor do
describe Gitlab::Email::Hook::AdditionalHeadersInterceptor do
let(:mail) do
ActionMailer::Base.mail(to: 'test@mail.com', from: 'info@mail.com', body: 'hello')
end
......
require 'spec_helper'
describe Gitlab::Email::Hook::DeliveryMetricsObserver do
let(:email) do
ActionMailer::Base.mail(to: 'test@example.com',
from: 'info@example.com',
body: 'hello')
end
context 'when email has been delivered' do
it 'increments both email delivery metrics' do
expect(described_class.delivery_attempts_counter).to receive(:increment)
expect(described_class.delivered_emails_counter).to receive(:increment)
email.deliver_now
end
end
context 'when email has not been delivered due to an error' do
before do
allow(email.delivery_method).to receive(:deliver!)
.and_raise(StandardError, 'Some SMTP error')
end
it 'increments only delivery attempt metric' do
expect(described_class.delivery_attempts_counter)
.to receive(:increment)
expect(described_class.delivered_emails_counter)
.not_to receive(:increment)
expect { email.deliver_now }
.to raise_error(StandardError, 'Some SMTP error')
end
end
end
require 'spec_helper'
describe DisableEmailInterceptor do
describe Gitlab::Email::Hook::DisableEmailInterceptor do
before do
Mail.register_interceptor(described_class)
end
......
......@@ -1366,7 +1366,8 @@ describe Notify do
it 'only sends the text template' do
stub_application_setting(html_emails_enabled: false)
EmailTemplateInterceptor.delivering_email(multipart_mail)
Gitlab::Email::Hook::EmailTemplateInterceptor
.delivering_email(multipart_mail)
expect(multipart_mail).to have_part_with('text/plain')
expect(multipart_mail).not_to have_part_with('text/html')
......@@ -1377,7 +1378,8 @@ describe Notify do
it 'sends a multipart message' do
stub_application_setting(html_emails_enabled: true)
EmailTemplateInterceptor.delivering_email(multipart_mail)
Gitlab::Email::Hook::EmailTemplateInterceptor
.delivering_email(multipart_mail)
expect(multipart_mail).to have_part_with('text/plain')
expect(multipart_mail).to have_part_with('text/html')
......
......@@ -197,6 +197,22 @@ describe ProjectWiki do
end
end
describe '#find_sidebar' do
before do
create_page(described_class::SIDEBAR, 'This is an awesome Sidebar')
end
after do
subject.pages.each { |page| destroy_page(page.page) }
end
it 'finds the page defined as _sidebar' do
page = subject.find_page('_sidebar')
expect(page.content).to eq('This is an awesome Sidebar')
end
end
describe '#find_file' do
shared_examples 'finding a wiki file' do
let(:image) { File.open(Rails.root.join('spec', 'fixtures', 'big-image.png')) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment