Commit 3592a597 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'master' into group-navigation-redesign

parents ccd9c4be c5834338
...@@ -953,10 +953,9 @@ Performance/DoubleStartEndWith: ...@@ -953,10 +953,9 @@ Performance/DoubleStartEndWith:
Performance/EndWith: Performance/EndWith:
Enabled: false Enabled: false
# TODO: Enable LstripRstrip Cop.
# Use `strip` instead of `lstrip.rstrip`. # Use `strip` instead of `lstrip.rstrip`.
Performance/LstripRstrip: Performance/LstripRstrip:
Enabled: false Enabled: true
# TODO: Enable RangeInclude Cop. # TODO: Enable RangeInclude Cop.
# Use `Range#cover?` instead of `Range#include?`. # Use `Range#cover?` instead of `Range#include?`.
......
...@@ -2,7 +2,9 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -2,7 +2,9 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.8.0 (unreleased) v 8.8.0 (unreleased)
- Project#open_branches has been cleaned up and no longer loads entire records into memory. - Project#open_branches has been cleaned up and no longer loads entire records into memory.
- Log to application.log when an admin starts and stops impersonating a user
- Make build status canceled if any of the jobs was canceled and none failed - Make build status canceled if any of the jobs was canceled and none failed
- Sanitize repo paths in new project error message
- Remove future dates from contribution calendar graph. - Remove future dates from contribution calendar graph.
- Support e-mail notifications for comments on project snippets - Support e-mail notifications for comments on project snippets
- Use ActionDispatch Remote IP for Akismet checking - Use ActionDispatch Remote IP for Akismet checking
...@@ -10,17 +12,26 @@ v 8.8.0 (unreleased) ...@@ -10,17 +12,26 @@ v 8.8.0 (unreleased)
- Add 'l' shortcut to open Label dropdown on issuables and 'i' to create new issue on a project - Add 'l' shortcut to open Label dropdown on issuables and 'i' to create new issue on a project
- Updated search UI - Updated search UI
- Display informative message when new milestone is created - Display informative message when new milestone is created
- Replace Devise Async with Devise ActiveJob integration. !3902 (Connor Shea)
- Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea) - Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea)
- Added button to toggle whitespaces changes on diff view - Added button to toggle whitespaces changes on diff view
- Backport GitLab Enterprise support from EE - Backport GitLab Enterprise support from EE
- Create tags using Rugged for performance reasons. !3745
- Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718 - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718
- Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes) - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
- Added multiple colors for labels in dropdowns when dups happen. - Added multiple colors for labels in dropdowns when dups happen.
- Improve description for the Two-factor Authentication sign-in screen. (Connor Shea)
- API support for the 'since' and 'until' operators on commit requests (Paco Guzman)
- Fix Gravatar hint in user profile when Gravatar is disabled. !3988 (Artem Sidorenko)
- Expire repository exists? and has_visible_content? caches after a push if necessary
v 8.7.2 (unreleased) v 8.7.3
- Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented
- Merge request widget displays TeamCity build state and code coverage correctly again.
v 8.7.2
- The "New Branch" button is now loaded asynchronously - The "New Branch" button is now loaded asynchronously
- Fix error 500 when trying to create a wiki page - Fix error 500 when trying to create a wiki page
- Updated spacing between notification label and button
v 8.7.1 v 8.7.1
- Throttle the update of `project.last_activity_at` to 1 minute. !3848 - Throttle the update of `project.last_activity_at` to 1 minute. !3848
......
...@@ -142,6 +142,16 @@ code snippet right after your description in a new line: `~"feature proposal"`. ...@@ -142,6 +142,16 @@ code snippet right after your description in a new line: `~"feature proposal"`.
Please keep feature proposals as small and simple as possible, complex ones Please keep feature proposals as small and simple as possible, complex ones
might be edited to make them small and simple. might be edited to make them small and simple.
You are encouraged to use the template below for feature proposals.
```
## Description including problem, use cases, benefits, and/or goals
## Proposal
## Links / references
```
For changes in the interface, it can be helpful to create a mockup first. For changes in the interface, it can be helpful to create a mockup first.
If you want to create something yourself, consider opening an issue first to If you want to create something yourself, consider opening an issue first to
discuss whether it is interesting to include this in GitLab. discuss whether it is interesting to include this in GitLab.
...@@ -349,7 +359,7 @@ on your merge request feel free to mention one of the Merge Marshalls in the ...@@ -349,7 +359,7 @@ on your merge request feel free to mention one of the Merge Marshalls in the
Please ensure that your merge request meets the contribution acceptance criteria. Please ensure that your merge request meets the contribution acceptance criteria.
When having your code reviewed and when reviewing merge requests please take the When having your code reviewed and when reviewing merge requests please take the
[Thoughtbot code review guide] into account. [code review guidelines](doc/development/code_review.md) into account.
### Merge request description format ### Merge request description format
...@@ -523,4 +533,3 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor ...@@ -523,4 +533,3 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor
[gitlab-design]: https://gitlab.com/gitlab-org/gitlab-design [gitlab-design]: https://gitlab.com/gitlab-org/gitlab-design
[free Antetype viewer (Mac OSX only)]: https://itunes.apple.com/us/app/antetype-viewer/id824152298?mt=12 [free Antetype viewer (Mac OSX only)]: https://itunes.apple.com/us/app/antetype-viewer/id824152298?mt=12
[`gitlab1.atype` file]: https://gitlab.com/gitlab-org/gitlab-design/tree/master/gitlab1.atype/ [`gitlab1.atype` file]: https://gitlab.com/gitlab-org/gitlab-design/tree/master/gitlab1.atype/
[Thoughtbot code review guide]: https://github.com/thoughtbot/guides/tree/master/code-review
...@@ -20,6 +20,7 @@ gem "pg", '~> 0.18.2', group: :postgres ...@@ -20,6 +20,7 @@ gem "pg", '~> 0.18.2', group: :postgres
# Authentication libraries # Authentication libraries
gem 'devise', '~> 3.5.4' gem 'devise', '~> 3.5.4'
gem 'doorkeeper', '~> 3.1' gem 'doorkeeper', '~> 3.1'
gem 'devise-async', '~> 0.9.0'
gem 'omniauth', '~> 1.3.1' gem 'omniauth', '~> 1.3.1'
gem 'omniauth-auth0', '~> 1.4.1' gem 'omniauth-auth0', '~> 1.4.1'
gem 'omniauth-azure-oauth2', '~> 0.0.6' gem 'omniauth-azure-oauth2', '~> 0.0.6'
...@@ -269,7 +270,7 @@ group :development, :test do ...@@ -269,7 +270,7 @@ group :development, :test do
gem 'database_cleaner', '~> 1.4.0' gem 'database_cleaner', '~> 1.4.0'
gem 'factory_girl_rails', '~> 4.6.0' gem 'factory_girl_rails', '~> 4.6.0'
gem 'rspec-rails', '~> 3.3.0' gem 'rspec-rails', '~> 3.4.0'
gem 'rspec-retry' gem 'rspec-retry'
gem 'spinach-rails', '~> 0.2.1' gem 'spinach-rails', '~> 0.2.1'
gem 'spinach-rerun-reporter', '~> 0.0.2' gem 'spinach-rerun-reporter', '~> 0.0.2'
......
...@@ -164,6 +164,8 @@ GEM ...@@ -164,6 +164,8 @@ GEM
responders responders
thread_safe (~> 0.1) thread_safe (~> 0.1)
warden (~> 1.2.3) warden (~> 1.2.3)
devise-async (0.9.0)
devise (~> 3.2)
devise-two-factor (2.0.1) devise-two-factor (2.0.1)
activesupport activesupport
attr_encrypted (~> 1.3.2) attr_encrypted (~> 1.3.2)
...@@ -351,7 +353,7 @@ GEM ...@@ -351,7 +353,7 @@ GEM
posix-spawn (~> 0.3) posix-spawn (~> 0.3)
gitlab_emoji (0.3.1) gitlab_emoji (0.3.1)
gemojione (~> 2.2, >= 2.2.1) gemojione (~> 2.2, >= 2.2.1)
gitlab_git (10.0.1) gitlab_git (10.0.2)
activesupport (~> 4.0) activesupport (~> 4.0)
charlock_holmes (~> 0.7.3) charlock_holmes (~> 0.7.3)
github-linguist (~> 4.7.0) github-linguist (~> 4.7.0)
...@@ -662,29 +664,29 @@ GEM ...@@ -662,29 +664,29 @@ GEM
chunky_png chunky_png
rqrcode-rails3 (0.1.7) rqrcode-rails3 (0.1.7)
rqrcode (>= 0.4.2) rqrcode (>= 0.4.2)
rspec (3.3.0) rspec (3.4.0)
rspec-core (~> 3.3.0) rspec-core (~> 3.4.0)
rspec-expectations (~> 3.3.0) rspec-expectations (~> 3.4.0)
rspec-mocks (~> 3.3.0) rspec-mocks (~> 3.4.0)
rspec-core (3.3.2) rspec-core (3.4.4)
rspec-support (~> 3.3.0) rspec-support (~> 3.4.0)
rspec-expectations (3.3.1) rspec-expectations (3.4.0)
diff-lcs (>= 1.2.0, < 2.0) diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.3.0) rspec-support (~> 3.4.0)
rspec-mocks (3.3.2) rspec-mocks (3.4.1)
diff-lcs (>= 1.2.0, < 2.0) diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.3.0) rspec-support (~> 3.4.0)
rspec-rails (3.3.3) rspec-rails (3.4.2)
actionpack (>= 3.0, < 4.3) actionpack (>= 3.0, < 4.3)
activesupport (>= 3.0, < 4.3) activesupport (>= 3.0, < 4.3)
railties (>= 3.0, < 4.3) railties (>= 3.0, < 4.3)
rspec-core (~> 3.3.0) rspec-core (~> 3.4.0)
rspec-expectations (~> 3.3.0) rspec-expectations (~> 3.4.0)
rspec-mocks (~> 3.3.0) rspec-mocks (~> 3.4.0)
rspec-support (~> 3.3.0) rspec-support (~> 3.4.0)
rspec-retry (0.4.5) rspec-retry (0.4.5)
rspec-core rspec-core
rspec-support (3.3.0) rspec-support (3.4.1)
rubocop (0.38.0) rubocop (0.38.0)
parser (>= 2.3.0.6, < 3.0) parser (>= 2.3.0.6, < 3.0)
powerpack (~> 0.1) powerpack (~> 0.1)
...@@ -920,6 +922,7 @@ DEPENDENCIES ...@@ -920,6 +922,7 @@ DEPENDENCIES
database_cleaner (~> 1.4.0) database_cleaner (~> 1.4.0)
default_value_for (~> 3.0.0) default_value_for (~> 3.0.0)
devise (~> 3.5.4) devise (~> 3.5.4)
devise-async (~> 0.9.0)
devise-two-factor (~> 2.0.0) devise-two-factor (~> 2.0.0)
diffy (~> 3.0.3) diffy (~> 3.0.3)
doorkeeper (~> 3.1) doorkeeper (~> 3.1)
...@@ -1011,7 +1014,7 @@ DEPENDENCIES ...@@ -1011,7 +1014,7 @@ DEPENDENCIES
responders (~> 2.0) responders (~> 2.0)
rouge (~> 1.10.1) rouge (~> 1.10.1)
rqrcode-rails3 (~> 0.1.7) rqrcode-rails3 (~> 0.1.7)
rspec-rails (~> 3.3.0) rspec-rails (~> 3.4.0)
rspec-retry rspec-retry
rubocop (~> 0.38.0) rubocop (~> 0.38.0)
ruby-fogbugz (~> 0.2.1) ruby-fogbugz (~> 0.2.1)
...@@ -1058,4 +1061,4 @@ DEPENDENCIES ...@@ -1058,4 +1061,4 @@ DEPENDENCIES
wikicloth (= 0.8.1) wikicloth (= 0.8.1)
BUNDLED WITH BUNDLED WITH
1.11.2 1.12.1
# GitLab # GitLab
[![build status](https://ci.gitlab.com/projects/1/status.svg?ref=master)](https://ci.gitlab.com/projects/1?ref=master) [![build status](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/build.svg)](https://gitlab.com/gitlab-org/gitlab-ce/commits/master)
[![Build Status](https://semaphoreci.com/api/v1/projects/2f1a5809-418b-4cc2-a1f4-819607579fe7/400484/shields_badge.svg)](https://semaphoreci.com/gitlabhq/gitlabhq) [![Build Status](https://semaphoreci.com/api/v1/projects/2f1a5809-418b-4cc2-a1f4-819607579fe7/400484/shields_badge.svg)](https://semaphoreci.com/gitlabhq/gitlabhq)
[![Code Climate](https://codeclimate.com/github/gitlabhq/gitlabhq.svg)](https://codeclimate.com/github/gitlabhq/gitlabhq) [![Code Climate](https://codeclimate.com/github/gitlabhq/gitlabhq.svg)](https://codeclimate.com/github/gitlabhq/gitlabhq)
[![Coverage Status](https://coveralls.io/repos/gitlabhq/gitlabhq/badge.svg?branch=master)](https://coveralls.io/r/gitlabhq/gitlabhq?branch=master) [![Coverage Status](https://coveralls.io/repos/gitlabhq/gitlabhq/badge.svg?branch=master)](https://coveralls.io/r/gitlabhq/gitlabhq?branch=master)
...@@ -80,7 +80,7 @@ There are a lot of [third-party applications integrating with GitLab](https://ab ...@@ -80,7 +80,7 @@ There are a lot of [third-party applications integrating with GitLab](https://ab
## GitLab release cycle ## GitLab release cycle
For more information about the release process see the [release documentation](http://doc.gitlab.com/ce/release/). For more information about the release process see the [release documentation](https://gitlab.com/gitlab-org/release-tools/blob/master/README.md).
## Upgrading ## Upgrading
......
...@@ -68,20 +68,18 @@ class @MergeRequestWidget ...@@ -68,20 +68,18 @@ class @MergeRequestWidget
$.getJSON @opts.ci_status_url, (data) => $.getJSON @opts.ci_status_url, (data) =>
@readyForCICheck = true @readyForCICheck = true
if @firstCICheck if data.status is ''
@firstCICheck = false
@opts.ci_status = data.status
if @opts.ci_status is ''
@opts.ci_status = data.status
return return
if data.status isnt @opts.ci_status and data.status? if @firstCiCheck || data.status isnt @opts.ci_status and data.status?
@opts.ci_status = data.status
@showCIStatus data.status @showCIStatus data.status
if data.coverage if data.coverage
@showCICoverage data.coverage @showCICoverage data.coverage
if showNotification # The first check should only update the UI, a notification
# should only be displayed on status changes
if showNotification and not @firstCiCheck
status = @ciLabelForStatus(data.status) status = @ciLabelForStatus(data.status)
if status is "preparing" if status is "preparing"
...@@ -104,8 +102,7 @@ class @MergeRequestWidget ...@@ -104,8 +102,7 @@ class @MergeRequestWidget
@close() @close()
Turbolinks.visit _this.opts.builds_path Turbolinks.visit _this.opts.builds_path
) )
@firstCiCheck = false
@opts.ci_status = data.status
showCIStatus: (state) -> showCIStatus: (state) ->
$('.ci_widget').hide() $('.ci_widget').hide()
......
...@@ -32,13 +32,11 @@ table { ...@@ -32,13 +32,11 @@ table {
th { th {
background-color: $background-color; background-color: $background-color;
font-weight: normal; font-weight: normal;
font-size: 15px; border-bottom: none;
border-bottom: 1px solid $border-color;
} }
td { td {
border-color: $table-border-color; border-color: $table-border-color;
border-bottom: 1px solid $border-color;
} }
} }
} }
......
...@@ -12,7 +12,7 @@ $gutter_inner_width: 258px; ...@@ -12,7 +12,7 @@ $gutter_inner_width: 258px;
*/ */
$border-color: #e5e5e5; $border-color: #e5e5e5;
$focus-border-color: #3aabf0; $focus-border-color: #3aabf0;
$table-border-color: #eef0f2; $table-border-color: #ececec;
$background-color: #fafafa; $background-color: #fafafa;
/* /*
......
...@@ -28,7 +28,7 @@ li.milestone { ...@@ -28,7 +28,7 @@ li.milestone {
// Issue title // Issue title
span a { span a {
color: rgba(0,0,0,0.64); color: $gl-text-color;
} }
} }
} }
...@@ -51,7 +51,7 @@ li.milestone { ...@@ -51,7 +51,7 @@ li.milestone {
margin-top: 7px; margin-top: 7px;
.issuable-number { .issuable-number {
color: rgba(0,0,0,0.44); color: $gl-placeholder-color;
margin-right: 5px; margin-right: 5px;
} }
.avatar { .avatar {
......
...@@ -114,10 +114,6 @@ ul.notes { ...@@ -114,10 +114,6 @@ ul.notes {
word-break: keep-all; word-break: keep-all;
} }
} }
a {
word-break: break-all;
}
} }
.note-header { .note-header {
......
...@@ -16,7 +16,7 @@ ...@@ -16,7 +16,7 @@
tr { tr {
> td, > th { > td, > th {
line-height: 26px; line-height: 23px;
} }
&:hover { &:hover {
......
...@@ -7,6 +7,8 @@ class Admin::ImpersonationsController < Admin::ApplicationController ...@@ -7,6 +7,8 @@ class Admin::ImpersonationsController < Admin::ApplicationController
warden.set_user(impersonator, scope: :user) warden.set_user(impersonator, scope: :user)
Gitlab::AppLogger.info("User #{original_user.username} has stopped impersonating #{impersonator.username}")
session[:impersonator_id] = nil session[:impersonator_id] = nil
redirect_to admin_user_path(original_user) redirect_to admin_user_path(original_user)
......
...@@ -41,6 +41,8 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -41,6 +41,8 @@ class Admin::UsersController < Admin::ApplicationController
warden.set_user(user, scope: :user) warden.set_user(user, scope: :user)
Gitlab::AppLogger.info("User #{current_user.username} has started impersonating #{user.username}")
flash[:alert] = "You are now impersonating #{user.username}" flash[:alert] = "You are now impersonating #{user.username}"
redirect_to root_path redirect_to root_path
......
...@@ -117,7 +117,7 @@ class ApplicationController < ActionController::Base ...@@ -117,7 +117,7 @@ class ApplicationController < ActionController::Base
end end
def after_sign_out_path_for(resource) def after_sign_out_path_for(resource)
current_application_settings.after_sign_out_path || new_user_session_path current_application_settings.after_sign_out_path.presence || new_user_session_path
end end
def abilities def abilities
......
...@@ -15,7 +15,7 @@ class Projects::CommitsController < Projects::ApplicationController ...@@ -15,7 +15,7 @@ class Projects::CommitsController < Projects::ApplicationController
if search.present? if search.present?
@repository.find_commits_by_message(search, @ref, @path, @limit, @offset).compact @repository.find_commits_by_message(search, @ref, @path, @limit, @offset).compact
else else
@repository.commits(@ref, @path, @limit, @offset) @repository.commits(@ref, path: @path, limit: @limit, offset: @offset)
end end
@note_counts = project.notes.where(commit_id: @commits.map(&:id)). @note_counts = project.notes.where(commit_id: @commits.map(&:id)).
......
...@@ -17,7 +17,7 @@ class Projects::GraphsController < Projects::ApplicationController ...@@ -17,7 +17,7 @@ class Projects::GraphsController < Projects::ApplicationController
end end
def commits def commits
@commits = @project.repository.commits(@ref, nil, 2000, 0, true) @commits = @project.repository.commits(@ref, limit: 2000, skip_merges: true)
@commits_graph = Gitlab::Graphs::Commits.new(@commits) @commits_graph = Gitlab::Graphs::Commits.new(@commits)
@commits_per_week_days = @commits_graph.commits_per_week_days @commits_per_week_days = @commits_graph.commits_per_week_days
@commits_per_time = @commits_graph.commits_per_time @commits_per_time = @commits_graph.commits_per_time
...@@ -55,7 +55,7 @@ class Projects::GraphsController < Projects::ApplicationController ...@@ -55,7 +55,7 @@ class Projects::GraphsController < Projects::ApplicationController
private private
def fetch_graph def fetch_graph
@commits = @project.repository.commits(@ref, nil, 6000, 0, true) @commits = @project.repository.commits(@ref, limit: 6000, skip_merges: true)
@log = [] @log = []
@commits.each do |commit| @commits.each do |commit|
......
...@@ -8,6 +8,13 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -8,6 +8,13 @@ class RegistrationsController < Devise::RegistrationsController
def create def create
if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
# To avoid duplicate form fields on the login page, the registration form
# names fields using `new_user`, but Devise still wants the params in
# `user`.
if params["new_#{resource_name}"].present? && params[resource_name].blank?
params[resource_name] = params.delete(:"new_#{resource_name}")
end
super super
else else
flash[:alert] = "There was an error with the reCAPTCHA code below. Please re-enter the code." flash[:alert] = "There was an error with the reCAPTCHA code below. Please re-enter the code."
......
...@@ -200,12 +200,8 @@ module ProjectsHelper ...@@ -200,12 +200,8 @@ module ProjectsHelper
end end
def repository_size(project = @project) def repository_size(project = @project)
"#{project.repository_size} MB" size_in_bytes = project.repository_size * 1.megabyte
rescue number_to_human_size(size_in_bytes, delimiter: ',', precision: 2)
# In order to prevent 500 error
# when application cannot allocate memory
# to calculate repo size - just show 'Unknown'
'unknown'
end end
def default_url_to_repo(project = @project) def default_url_to_repo(project = @project)
...@@ -341,4 +337,10 @@ module ProjectsHelper ...@@ -341,4 +337,10 @@ module ProjectsHelper
) )
end end
end end
def sanitize_repo_path(message)
return '' unless message.present?
message.strip.gsub(Gitlab.config.gitlab_shell.repos_path.chomp('/'), "[REPOS PATH]")
end
end end
...@@ -87,13 +87,15 @@ class Repository ...@@ -87,13 +87,15 @@ class Repository
nil nil
end end
def commits(ref, path = nil, limit = nil, offset = nil, skip_merges = false) def commits(ref, path: nil, limit: nil, offset: nil, skip_merges: false, after: nil, before: nil)
options = { options = {
repo: raw_repository, repo: raw_repository,
ref: ref, ref: ref,
path: path, path: path,
limit: limit, limit: limit,
offset: offset, offset: offset,
after: after,
before: before,
# --follow doesn't play well with --skip. See: # --follow doesn't play well with --skip. See:
# https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520 # https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520
follow: false, follow: false,
...@@ -146,10 +148,20 @@ class Repository ...@@ -146,10 +148,20 @@ class Repository
find_branch(branch_name) find_branch(branch_name)
end end
def add_tag(tag_name, ref, message = nil) def add_tag(user, tag_name, target, message = nil)
before_push_tag oldrev = Gitlab::Git::BLANK_SHA
ref = Gitlab::Git::TAG_REF_PREFIX + tag_name
target = commit(target).try(:id)
return false unless target
options = { message: message, tagger: user_to_committer(user) } if message
GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do
rugged.tags.create(tag_name, target, options)
end
gitlab_shell.add_tag(path_with_namespace, tag_name, ref, message) find_tag(tag_name)
end end
def rm_branch(user, branch_name) def rm_branch(user, branch_name)
...@@ -575,7 +587,7 @@ class Repository ...@@ -575,7 +587,7 @@ class Repository
end end
def contributors def contributors
commits = self.commits(nil, nil, 2000, 0, true) commits = self.commits(nil, limit: 2000, offset: 0, skip_merges: true)
commits.group_by(&:author_email).map do |email, commits| commits.group_by(&:author_email).map do |email, commits|
contributor = Gitlab::Contributor.new contributor = Gitlab::Contributor.new
......
...@@ -91,7 +91,7 @@ class User < ActiveRecord::Base ...@@ -91,7 +91,7 @@ class User < ActiveRecord::Base
devise :two_factor_backupable, otp_number_of_backup_codes: 10 devise :two_factor_backupable, otp_number_of_backup_codes: 10
serialize :otp_backup_codes, JSON serialize :otp_backup_codes, JSON
devise :lockable, :recoverable, :rememberable, :trackable, devise :lockable, :async, :recoverable, :rememberable, :trackable,
:validatable, :omniauthable, :confirmable, :registerable :validatable, :omniauthable, :confirmable, :registerable
attr_accessor :force_random_password attr_accessor :force_random_password
......
...@@ -43,9 +43,4 @@ class CreateBranchService < BaseService ...@@ -43,9 +43,4 @@ class CreateBranchService < BaseService
out[:branch] = branch out[:branch] = branch
out out
end end
def build_push_data(project, user, branch)
Gitlab::PushDataBuilder.
build(project, user, Gitlab::Git::BLANK_SHA, branch.target, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", [])
end
end end
require_relative 'base_service' require_relative 'base_service'
class CreateTagService < BaseService class CreateTagService < BaseService
def execute(tag_name, ref, message, release_description = nil) def execute(tag_name, target, message, release_description = nil)
valid_tag = Gitlab::GitRefValidator.validate(tag_name) valid_tag = Gitlab::GitRefValidator.validate(tag_name)
if valid_tag == false return error('Tag name invalid') unless valid_tag
return error('Tag name invalid')
end
repository = project.repository repository = project.repository
existing_tag = repository.find_tag(tag_name)
if existing_tag
return error('Tag already exists')
end
message.strip! if message message.strip! if message
repository.add_tag(tag_name, ref, message) new_tag = nil
new_tag = repository.find_tag(tag_name) begin
new_tag = repository.add_tag(current_user, tag_name, target, message)
rescue Rugged::TagError
return error("Tag #{tag_name} already exists")
rescue GitHooksService::PreReceiveError
return error('Tag creation was rejected by Git hook')
end
if new_tag if new_tag
push_data = create_push_data(project, current_user, new_tag)
EventCreateService.new.push(project, current_user, push_data)
project.execute_hooks(push_data.dup, :tag_push_hooks)
project.execute_services(push_data.dup, :tag_push_hooks)
CreateCommitBuildsService.new.execute(project, current_user, push_data)
if release_description if release_description
CreateReleaseService.new(@project, @current_user). CreateReleaseService.new(@project, @current_user).
execute(tag_name, release_description) execute(tag_name, release_description)
end end
success.merge(tag: new_tag)
success(new_tag)
else else
error('Invalid reference name') error("Target #{target} is invalid")
end
end end
def success(branch)
out = super()
out[:tag] = branch
out
end
def create_push_data(project, user, tag)
commits = [project.commit(tag.target)].compact
Gitlab::PushDataBuilder.
build(project, user, Gitlab::Git::BLANK_SHA, tag.target, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", commits, tag.message)
end end
end end
...@@ -17,6 +17,7 @@ class GitPushService < BaseService ...@@ -17,6 +17,7 @@ class GitPushService < BaseService
# 6. Checks if the project's main language has changed # 6. Checks if the project's main language has changed
# #
def execute def execute
@project.repository.after_create if @project.empty_repo?
@project.repository.after_push_commit(branch_name, params[:newrev]) @project.repository.after_push_commit(branch_name, params[:newrev])
if push_remove_branch? if push_remove_branch?
......
...@@ -2,6 +2,7 @@ class GitTagPushService < BaseService ...@@ -2,6 +2,7 @@ class GitTagPushService < BaseService
attr_accessor :push_data attr_accessor :push_data
def execute def execute
project.repository.after_create if project.empty_repo?
project.repository.before_push_tag project.repository.before_push_tag
@push_data = build_push_data @push_data = build_push_data
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
%h3 Two-factor Authentication %h3 Two-factor Authentication
.login-body .login-body
= form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f| = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f|
= f.text_field :otp_attempt, class: 'form-control', placeholder: 'Two-factor authentication code', required: true, autofocus: true = f.text_field :otp_attempt, class: 'form-control', placeholder: 'Two-factor Authentication code', required: true, autofocus: true
%p.help-block.hint If you've lost your phone, you may enter one of your recovery codes. %p.help-block.hint Enter the code from the two-factor app on your mobile device. If you've lost your device, you may enter one of your recovery codes.
.prepend-top-20 .prepend-top-20
= f.submit "Verify code", class: "btn btn-save" = f.submit "Verify code", class: "btn btn-save"
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
.login-heading .login-heading
%h3 Create an account %h3 Create an account
.login-body .login-body
= form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f| = form_for(resource, as: "new_#{resource_name}", url: registration_path(resource_name)) do |f|
.devise-errors .devise-errors
= devise_error_messages! = devise_error_messages!
%div %div
...@@ -16,7 +16,7 @@ ...@@ -16,7 +16,7 @@
%div %div
= f.email_field :email, class: "form-control middle", placeholder: "Email", required: true = f.email_field :email, class: "form-control middle", placeholder: "Email", required: true
.form-group.append-bottom-20#password-strength .form-group.append-bottom-20#password-strength
= f.password_field :password, class: "form-control bottom", id: "user_password_sign_up", placeholder: "Password", required: true = f.password_field :password, class: "form-control bottom", placeholder: "Password", required: true
%div %div
- if current_application_settings.recaptcha_enabled - if current_application_settings.recaptcha_enabled
= recaptcha_tags = recaptcha_tags
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
= cache [event, current_application_settings, "v2.2"] do = cache [event, current_application_settings, "v2.2"] do
- if event.author - if event.author
= link_to user_path(event.author.username) do = link_to user_path(event.author) do
= image_tag avatar_icon(event.author_email, 40), class: "avatar s40", alt:'' = image_tag avatar_icon(event.author_email, 40), class: "avatar s40", alt:''
- else - else
= image_tag avatar_icon(event.author_email, 40), class: "avatar s40", alt:'' = image_tag avatar_icon(event.author_email, 40), class: "avatar s40", alt:''
......
...@@ -8,11 +8,11 @@ ...@@ -8,11 +8,11 @@
%p %p
- if @user.avatar? - if @user.avatar?
You can change your avatar here You can change your avatar here
- if Gitlab.config.gravatar.enabled - if gravatar_enabled?
or remove the current avatar to revert to #{link_to Gitlab.config.gravatar.host, "http://" + Gitlab.config.gravatar.host} or remove the current avatar to revert to #{link_to Gitlab.config.gravatar.host, "http://" + Gitlab.config.gravatar.host}
- else - else
You can upload an avatar here You can upload an avatar here
- if Gitlab.config.gravatar.enabled - if gravatar_enabled?
or change it at #{link_to Gitlab.config.gravatar.host, "http://" + Gitlab.config.gravatar.host} or change it at #{link_to Gitlab.config.gravatar.host, "http://" + Gitlab.config.gravatar.host}
.col-lg-9 .col-lg-9
.clearfix.avatar-image.append-bottom-default .clearfix.avatar-image.append-bottom-default
......
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
.panel-body .panel-body
%pre %pre
:preserve :preserve
#{@project.import_error.try(:strip)} #{sanitize_repo_path(@project.import_error)}
= form_for @project, url: namespace_project_import_path(@project.namespace, @project), method: :post, html: { class: 'form-horizontal' } do |f| = form_for @project, url: namespace_project_import_path(@project.namespace, @project), method: :post, html: { class: 'form-horizontal' } do |f|
= render "shared/import_form", f: f = render "shared/import_form", f: f
......
...@@ -24,15 +24,15 @@ ...@@ -24,15 +24,15 @@
- else - else
= link_to 'Reopen Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-nr btn-grouped" = link_to 'Reopen Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-nr btn-grouped"
= link_to namespace_project_milestone_path(@project.namespace, @project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-nr" do
= icon('trash-o')
Delete
= link_to edit_namespace_project_milestone_path(@project.namespace, @project, @milestone), class: "btn btn-grouped btn-nr" do = link_to edit_namespace_project_milestone_path(@project.namespace, @project, @milestone), class: "btn btn-grouped btn-nr" do
= icon('pencil-square-o') = icon('pencil-square-o')
Edit Edit
.detail-page-description.milestone-detail.second-block = link_to namespace_project_milestone_path(@project.namespace, @project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-danger" do
= icon('trash-o')
Delete
.detail-page-description.milestone-detail
%h2.title %h2.title
= markdown escape_once(@milestone.title), pipeline: :single_line = markdown escape_once(@milestone.title), pipeline: :single_line
%div %div
......
...@@ -24,7 +24,7 @@ ...@@ -24,7 +24,7 @@
- else - else
= link_to 'Reopen Milestone', group_milestone_path(group, milestone.safe_title, title: milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-grouped btn-reopen" = link_to 'Reopen Milestone', group_milestone_path(group, milestone.safe_title, title: milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-grouped btn-reopen"
.detail-page-description.gray-content-block.second-block .detail-page-description.milestone-detail
%h2.title %h2.title
= markdown escape_once(milestone.title), pipeline: :single_line = markdown escape_once(milestone.title), pipeline: :single_line
......
...@@ -98,7 +98,7 @@ ...@@ -98,7 +98,7 @@
#groups.tab-pane #groups.tab-pane
- # This tab is always loaded via AJAX - # This tab is always loaded via AJAX
#contributed.contributed-projects.tab-pane #contributed.tab-pane
- # This tab is always loaded via AJAX - # This tab is always loaded via AJAX
#projects.tab-pane #projects.tab-pane
......
...@@ -15,13 +15,25 @@ module RepositoryCheck ...@@ -15,13 +15,25 @@ module RepositoryCheck
private private
def check(project) def check(project)
repositories = [project.repository] if !git_fsck(project.repository)
repositories << project.wiki.repository if project.wiki_enabled? false
# Use 'map do', not 'all? do', to prevent short-circuiting elsif project.wiki_enabled?
repositories.map { |repository| git_fsck(repository.path_to_repo) }.all? # Historically some projects never had their wiki repos initialized;
# this happens on project creation now. Let's initialize an empty repo
# if it is not already there.
begin
project.create_wiki
rescue Rugged::RepositoryError
end
git_fsck(project.wiki.repository)
else
true
end
end end
def git_fsck(path) def git_fsck(repository)
path = repository.path_to_repo
cmd = %W(nice git --git-dir=#{path} fsck) cmd = %W(nice git --git-dir=#{path} fsck)
output, status = Gitlab::Popen.popen(cmd) output, status = Gitlab::Popen.popen(cmd)
......
...@@ -37,7 +37,7 @@ start_no_deamonize() ...@@ -37,7 +37,7 @@ start_no_deamonize()
start_sidekiq() start_sidekiq()
{ {
bundle exec sidekiq -q post_receive -q mailers -q archive_repo -q system_hook -q project_web_hook -q gitlab_shell -q incoming_email -q runner -q common -q default -e $RAILS_ENV -P $sidekiq_pidfile "$@" exec bundle exec sidekiq -q post_receive -q mailers -q archive_repo -q system_hook -q project_web_hook -q gitlab_shell -q incoming_email -q runner -q common -q default -e $RAILS_ENV -P $sidekiq_pidfile "$@"
} }
load_ok() load_ok()
......
...@@ -19,12 +19,12 @@ get_unicorn_pid() ...@@ -19,12 +19,12 @@ get_unicorn_pid()
start() start()
{ {
$unicorn_cmd -D exec $unicorn_cmd -D
} }
start_foreground() start_foreground()
{ {
$unicorn_cmd exec $unicorn_cmd
} }
stop() stop()
......
...@@ -152,7 +152,6 @@ production: &base ...@@ -152,7 +152,6 @@ production: &base
## Gravatar ## Gravatar
## For Libravatar see: http://doc.gitlab.com/ce/customization/libravatar.html ## For Libravatar see: http://doc.gitlab.com/ce/customization/libravatar.html
gravatar: gravatar:
enabled: true # Use user avatar image from Gravatar.com (default: true)
# gravatar urls: possible placeholders: %{hash} %{size} %{email} # gravatar urls: possible placeholders: %{hash} %{size} %{email}
# plain_url: "http://..." # default: http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon # plain_url: "http://..." # default: http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon
# ssl_url: "https://..." # default: https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon # ssl_url: "https://..." # default: https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon
......
Devise::Async.backend = :sidekiq
...@@ -61,11 +61,29 @@ if Gitlab::Metrics.enabled? ...@@ -61,11 +61,29 @@ if Gitlab::Metrics.enabled?
config.instrument_instance_methods(const) config.instrument_instance_methods(const)
end end
Dir[Rails.root.join('app', 'finders', '*.rb')].each do |path| # Path to search => prefix to strip from constant
const = File.basename(path, '.rb').camelize.constantize paths_to_instrument = {
['app', 'finders'] => ['app', 'finders'],
['app', 'mailers', 'emails'] => ['app', 'mailers'],
['app', 'services', '**'] => ['app', 'services'],
['lib', 'gitlab', 'diff'] => ['lib'],
['lib', 'gitlab', 'email', 'message'] => ['lib']
}
paths_to_instrument.each do |(path, prefix)|
prefix = Rails.root.join(*prefix)
Dir[Rails.root.join(*path + ['*.rb'])].each do |file_path|
path = Pathname.new(file_path).relative_path_from(prefix)
const = path.to_s.sub('.rb', '').camelize.constantize
config.instrument_methods(const)
config.instrument_instance_methods(const) config.instrument_instance_methods(const)
end end
end
config.instrument_methods(Premailer::Adapter::Nokogiri)
config.instrument_instance_methods(Premailer::Adapter::Nokogiri)
[ [
:Blame, :Branch, :BranchCollection, :Blob, :Commit, :Diff, :Repository, :Blame, :Branch, :BranchCollection, :Blob, :Commit, :Diff, :Repository,
...@@ -97,17 +115,6 @@ if Gitlab::Metrics.enabled? ...@@ -97,17 +115,6 @@ if Gitlab::Metrics.enabled?
config.instrument_methods(Gitlab::ReferenceExtractor) config.instrument_methods(Gitlab::ReferenceExtractor)
config.instrument_instance_methods(Gitlab::ReferenceExtractor) config.instrument_instance_methods(Gitlab::ReferenceExtractor)
# Instrument all service classes
services = Rails.root.join('app', 'services')
Dir[services.join('**', '*.rb')].each do |file_path|
path = Pathname.new(file_path).relative_path_from(services)
const = path.to_s.sub('.rb', '').camelize.constantize
config.instrument_methods(const)
config.instrument_instance_methods(const)
end
# Instrument the classes used for checking if somebody has push access. # Instrument the classes used for checking if somebody has push access.
config.instrument_instance_methods(Gitlab::GitAccess) config.instrument_instance_methods(Gitlab::GitAccess)
config.instrument_instance_methods(Gitlab::GitAccessWiki) config.instrument_instance_methods(Gitlab::GitAccessWiki)
......
Gitlab::Seeder.quiet do Gitlab::Seeder.quiet do
# Limit the number of merge requests per project to avoid long seeds
MAX_NUM_MERGE_REQUESTS = 10
Project.all.reject(&:empty_repo?).each do |project| Project.all.reject(&:empty_repo?).each do |project|
branches = project.repository.branch_names branches = project.repository.branch_names.sample(MAX_NUM_MERGE_REQUESTS * 2)
branches.each do |branch_name| branches.each do |branch_name|
break if branches.size < 2 break if branches.size < 2
......
...@@ -12,6 +12,8 @@ GET /projects/:id/repository/commits ...@@ -12,6 +12,8 @@ GET /projects/:id/repository/commits
| --------- | ---- | -------- | ----------- | | --------- | ---- | -------- | ----------- |
| `id` | integer | yes | The ID of a project | | `id` | integer | yes | The ID of a project |
| `ref_name` | string | no | The name of a repository branch or tag or if not given the default branch | | `ref_name` | string | no | The name of a repository branch or tag or if not given the default branch |
| `since` | string | no | Only commits after or in this date will be returned in ISO 8601 format YYYY-MM-DDTHH:MM:SSZ |
| `until` | string | no | Only commits before or in this date will be returned in ISO 8601 format YYYY-MM-DDTHH:MM:SSZ |
```bash ```bash
curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/repository/commits" curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/repository/commits"
......
...@@ -15,7 +15,7 @@ GET /projects/:id/issues/:issue_id/notes ...@@ -15,7 +15,7 @@ GET /projects/:id/issues/:issue_id/notes
Parameters: Parameters:
- `id` (required) - The ID of a project - `id` (required) - The ID of a project
- `issue_id` (required) - The ID of an issue - `issue_id` (required) - The IID of an issue (not ID)
```json ```json
[ [
...@@ -73,7 +73,7 @@ GET /projects/:id/issues/:issue_id/notes/:note_id ...@@ -73,7 +73,7 @@ GET /projects/:id/issues/:issue_id/notes/:note_id
Parameters: Parameters:
- `id` (required) - The ID of a project - `id` (required) - The ID of a project
- `issue_id` (required) - The ID of a project issue - `issue_id` (required) - The IID of a project issue (not ID)
- `note_id` (required) - The ID of an issue note - `note_id` (required) - The ID of an issue note
### Create new issue note ### Create new issue note
...@@ -87,7 +87,7 @@ POST /projects/:id/issues/:issue_id/notes ...@@ -87,7 +87,7 @@ POST /projects/:id/issues/:issue_id/notes
Parameters: Parameters:
- `id` (required) - The ID of a project - `id` (required) - The ID of a project
- `issue_id` (required) - The ID of an issue - `issue_id` (required) - The IID of an issue (not ID)
- `body` (required) - The content of a note - `body` (required) - The content of a note
- `created_at` (optional) - Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z - `created_at` (optional) - Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z
...@@ -102,7 +102,7 @@ PUT /projects/:id/issues/:issue_id/notes/:note_id ...@@ -102,7 +102,7 @@ PUT /projects/:id/issues/:issue_id/notes/:note_id
Parameters: Parameters:
- `id` (required) - The ID of a project - `id` (required) - The ID of a project
- `issue_id` (required) - The ID of an issue - `issue_id` (required) - The IID of an issue (not ID)
- `note_id` (required) - The ID of a note - `note_id` (required) - The ID of a note
- `body` (required) - The content of a note - `body` (required) - The content of a note
...@@ -120,7 +120,7 @@ Parameters: ...@@ -120,7 +120,7 @@ Parameters:
| Attribute | Type | Required | Description | | Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- | | --------- | ---- | -------- | ----------- |
| `id` | integer | yes | The ID of a project | | `id` | integer | yes | The ID of a project |
| `issue_id` | integer | yes | The ID of an issue | | `issue_id` | integer | yes | The IID of an issue |
| `note_id` | integer | yes | The ID of a note | | `note_id` | integer | yes | The ID of a note |
```bash ```bash
......
...@@ -57,7 +57,7 @@ before_script: ...@@ -57,7 +57,7 @@ before_script:
# WARNING: Use this only with the Docker executor, if you use it with shell # WARNING: Use this only with the Docker executor, if you use it with shell
# you will overwrite your user's SSH config. # you will overwrite your user's SSH config.
- mkdir -p ~/.ssh - mkdir -p ~/.ssh
- '[[ -f /.dockerinit ]] && echo -e "Host *\n\tStrictHostKeyChecking no\n\n" > ~/.ssh/config' - '[[ -f /.dockerenv ]] && echo -e "Host *\n\tStrictHostKeyChecking no\n\n" > ~/.ssh/config'
``` ```
As a final step, add the _public_ key from the one you created earlier to the As a final step, add the _public_ key from the one you created earlier to the
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
- [How to dump production data to staging](db_dump.md) - [How to dump production data to staging](db_dump.md)
- [Instrumentation](instrumentation.md) - [Instrumentation](instrumentation.md)
- [Migration Style Guide](migration_style_guide.md) for creating safe migrations - [Migration Style Guide](migration_style_guide.md) for creating safe migrations
- [Performance guidelines](performance.md)
- [Rake tasks](rake_tasks.md) for development - [Rake tasks](rake_tasks.md) for development
- [Shell commands](shell_commands.md) in the GitLab codebase - [Shell commands](shell_commands.md) in the GitLab codebase
- [Sidekiq debugging](sidekiq_debugging.md) - [Sidekiq debugging](sidekiq_debugging.md)
......
# Performance Guidelines
This document describes various guidelines to follow to ensure good and
consistent performance of GitLab.
## Workflow
The process of solving performance problems is roughly as follows:
1. Make sure there's an issue open somewhere (e.g., on the GitLab CE issue
tracker), create one if there isn't. See [#15607][#15607] for an example.
2. Measure the performance of the code in a production environment such as
GitLab.com (see the [Tooling](#tooling) section below). Performance should be
measured over a period of _at least_ 24 hours.
3. Add your findings based on the measurement period (screenshots of graphs,
timings, etc) to the issue mentioned in step 1.
4. Solve the problem.
5. Create a merge request, assign the "performance" label and ping the right
people (e.g. [@yorickpeterse][yorickpeterse] and [@joshfng][joshfng]).
6. Once a change has been deployed make sure to _again_ measure for at least 24
hours to see if your changes have any impact on the production environment.
7. Repeat until you're done.
When providing timings make sure to provide:
* The 95th percentile
* The 99th percentile
* The mean
When providing screenshots of graphs, make sure that both the X and Y axes and
the legend are clearly visible. If you happen to have access to GitLab.com's own
monitoring tools you should also provide a link to any relevant
graphs/dashboards.
## Tooling
GitLab provides two built-in tools to aid the process of improving performance:
* [Sherlock](doc/development/profiling.md#sherlock)
* [GitLab Performance Monitoring](doc/monitoring/performance/monitoring.md)
GitLab employees can use GitLab.com's performance monitoring systems located at
<http://performance.gitlab.net>, this requires you to log in using your
`@gitlab.com` Email address. Non-GitLab employees are advised to set up their
own InfluxDB + Grafana stack.
## Benchmarks
Benchmarks are almost always useless. Benchmarks usually only test small bits of
code in isolation and often only measure the best case scenario. On top of that,
benchmarks for libraries (e.g., a Gem) tend to be biased in favour of the
library. After all there's little benefit to an author publishing a benchmark
that shows they perform worse than their competitors.
Benchmarks are only really useful when you need a rough (emphasis on "rough")
understanding of the impact of your changes. For example, if a certain method is
slow a benchmark can be used to see if the changes you're making have any impact
on the method's performance. However, even when a benchmark shows your changes
improve performance there's no guarantee the performance also improves in a
production environment.
When writing benchmarks you should almost always use
[benchmark-ips](https://github.com/evanphx/benchmark-ips). Ruby's `Benchmark`
module that comes with the standard library is rarely useful as it runs either a
single iteration (when using `Benchmark.bm`) or two iterations (when using
`Benchmark.bmbm`). Running this few iterations means external factors (e.g. a
video streaming in the background) can very easily skew the benchmark
statistics.
Another problem with the `Benchmark` module is that it displays timings, not
iterations. This means that if a piece of code completes in a very short period
of time it can be very difficult to compare the timings before and after a
certain change. This in turn leads to patterns such as the following:
```ruby
Benchmark.bmbm(10) do |bench|
bench.report 'do something' do
100.times do
... work here ...
end
end
end
```
This however leads to the question: how many iterations should we run to get
meaningful statistics?
The benchmark-ips Gem basically takes care of all this and much more, and as a
result of this should be used instead of the `Benchmark` module.
In short:
1. Don't trust benchmarks you find on the internet.
2. Never make claims based on just benchmarks, always measure in production to
confirm your findings.
3. X being N times faster than Y is meaningless if you don't know what impact it
will actually have on your production environment.
4. A production environment is the _only_ benchmark that always tells the truth
(unless your performance monitoring systems are not set up correctly).
5. If you must write a benchmark use the benchmark-ips Gem instead of Ruby's
`Benchmark` module.
## Importance of Changes
When working on performance improvements, it's important to always ask yourself
the question "How important is it to improve the performance of this piece of
code?". Not every piece of code is equally important and it would be a waste to
spend a week trying to improve something that only impacts a tiny fraction of
our users. For example, spending a week trying to squeeze 10 milliseconds out of
a method is a waste of time when you could have spent a week squeezing out 10
seconds elsewhere.
There is no clear set of steps that you can follow to determine if a certain
piece of code is worth optimizing. The only two things you can do are:
1. Think about what the code does, how it's used, how many times it's called and
how much time is spent in it relative to the total execution time (e.g., the
total time spent in a web request).
2. Ask others (preferably in the form of an issue).
Some examples of changes that aren't really important/worth the effort:
* Replacing double quotes with single quotes.
* Replacing usage of Array with Set when the list of values is very small.
* Replacing library A with library B when both only take up 0.1% of the total
execution time.
* Calling `freeze` on every string (see [String Freezing](#string-freezing)).
## Slow Operations & Sidekiq
Slow operations (e.g. merging branches) or operations that are prone to errors
(using external APIs) should be performed in a Sidekiq worker instead of
directly in a web request as much as possible. This has numerous benefits such
as:
1. An error won't prevent the request from completing.
2. The process being slow won't affect the loading time of a page.
3. In case of a failure it's easy to re-try the process (Sidekiq takes care of
this automatically).
4. By isolating the code from a web request it will hopefully be easier to test
and maintain.
It's especially important to use Sidekiq as much as possible when dealing with
Git operations as these operations can take quite some time to complete
depending on the performance of the underlying storage system.
## Git Operations
Care should be taken to not run unnecessary Git operations. For example,
retrieving the list of branch names using `Repository#branch_names` can be done
without an explicit check if a repository exists or not. In other words, instead
of this:
```ruby
if repository.exists?
repository.branch_names.each do |name|
...
end
end
```
You can just write:
```ruby
repository.branch_names.each do |name|
...
end
```
## Caching
Operations that will often return the same result should be cached using Redis,
in particular Git operations. When caching data in Redis, make sure the cache is
flushed whenever needed. For example, a cache for the list of tags should be
flushed whenever a new tag is pushed or a tag is removed.
When adding cache expiration code for repositories, this code should be placed
in one of the before/after hooks residing in the Repository class. For example,
if a cache should be flushed after importing a repository this code should be
added to `Repository#after_import`. This ensures the cache logic stays within
the Repository class instead of leaking into other classes.
When caching data, make sure to also memoize the result in an instance variable.
While retrieving data from Redis is much faster than raw Git operations, it still
has overhead. By caching the result in an instance variable, repeated calls to
the same method won't end up retrieving data from Redis upon every call. When
memoizing cached data in an instance variable, make sure to also reset the
instance variable when flushing the cache. An example:
```ruby
def first_branch
@first_branch ||= cache.fetch(:first_branch) { branches.first }
end
def expire_first_branch_cache
cache.expire(:first_branch)
@first_branch = nil
end
```
## Anti-Patterns
This is a collection of [anti-patterns][anti-pattern] that should be avoided
unless these changes have a measurable, significant and positive impact on
production environments.
### String Freezing
In recent Ruby versions calling `freeze` on a String leads to it being allocated
only once and re-used. For example, on Ruby 2.3 this will only allocate the
"foo" String once:
```ruby
10.times do
'foo'.freeze
end
```
Blindly adding a `.freeze` call to every String is an anti-pattern that should
be avoided unless one can prove (using production data) the call actually has a
positive impact on performance.
This feature of Ruby wasn't really meant to make things faster directly, instead
it was meant to reduce the number of allocations. Depending on the size of the
String and how frequently it would be allocated (before the `.freeze` call was
added), this _may_ make things faster, but there's no guarantee it will.
Another common flavour of this is to not only freeze a String, but also assign
it to a constant, for example:
```ruby
SOME_CONSTANT = 'foo'.freeze
9000.times do
SOME_CONSTANT
end
```
The only reason you should be doing this is to prevent somebody from mutating
the global String. However, since you can just re-assign constants in Ruby
there's nothing stopping somebody from doing this elsewhere in the code:
```ruby
SOME_CONSTANT = 'bar'
```
### Moving Allocations to Constants
Storing an object as a constant so you only allocate it once _may_ improve
performance, but there's no guarantee this will. Looking up constants has an
impact on runtime performance, and as such, using a constant instead of
referencing an object directly may even slow code down.
[#15607]: https://gitlab.com/gitlab-org/gitlab-ce/issues/15607
[yorickpeterse]: https://gitlab.com/u/yorickpeterse
[joshfng]: https://gitlab.com/u/joshfng
[anti-pattern]: https://en.wikipedia.org/wiki/Anti-pattern
...@@ -157,22 +157,64 @@ Create a `git` user for GitLab: ...@@ -157,22 +157,64 @@ Create a `git` user for GitLab:
## 5. Database ## 5. Database
We recommend using a PostgreSQL database. For MySQL check [MySQL setup guide](database_mysql.md). *Note*: because we need to make use of extensions you need at least pgsql 9.1. We recommend using a PostgreSQL database. For MySQL check the
[MySQL setup guide](database_mysql.md).
# Install the database packages > **Note**: because we need to make use of extensions you need at least pgsql 9.1.
sudo apt-get install -y postgresql postgresql-client libpq-dev
# Create a user for GitLab 1. Install the database packages:
```bash
sudo apt-get install -y postgresql postgresql-client libpq-dev postgresql-contrib
```
1. Create a database user for GitLab:
```bash
sudo -u postgres psql -d template1 -c "CREATE USER git CREATEDB;" sudo -u postgres psql -d template1 -c "CREATE USER git CREATEDB;"
```
1. Create the GitLab production database and grant all privileges on database:
# Create the GitLab production database & grant all privileges on database ```bash
sudo -u postgres psql -d template1 -c "CREATE DATABASE gitlabhq_production OWNER git;" sudo -u postgres psql -d template1 -c "CREATE DATABASE gitlabhq_production OWNER git;"
```
1. Create the `pg_trgm` extension (required for GitLab 8.6+):
```bash
sudo -u postgres psql -d template1 -c "CREATE EXTENSION IF NOT EXISTS pg_trgm;"
```
1. Try connecting to the new database with the new user:
# Try connecting to the new database with the new user ```bash
sudo -u git -H psql -d gitlabhq_production sudo -u git -H psql -d gitlabhq_production
```
1. Check if the `pg_trgm` extension is enabled:
```bash
SELECT true AS enabled
FROM pg_available_extensions
WHERE name = 'pg_trgm'
AND installed_version IS NOT NULL;
```
If the extension is enabled this will produce the following output:
# Quit the database session ```
enabled
---------
t
(1 row)
```
1. Quit the database session:
```bash
gitlabhq_production> \q gitlabhq_production> \q
```
## 6. Redis ## 6. Redis
......
...@@ -57,10 +57,10 @@ sudo -u git -H make ...@@ -57,10 +57,10 @@ sudo -u git -H make
cd /home/git/gitlab cd /home/git/gitlab
# PostgreSQL # PostgreSQL
sudo -u git -H bundle install --without development test mysql --deployment sudo -u git -H bundle install --without development test mysql --with postgres --deployment
# MySQL # MySQL
sudo -u git -H bundle install --without development test postgres --deployment sudo -u git -H bundle install --without development test postgres --with mysql --deployment
# Optional: clean up old gems # Optional: clean up old gems
sudo -u git -H bundle clean sudo -u git -H bundle clean
......
...@@ -57,11 +57,11 @@ class Spinach::Features::ProjectCommitsTags < Spinach::FeatureSteps ...@@ -57,11 +57,11 @@ class Spinach::Features::ProjectCommitsTags < Spinach::FeatureSteps
end end
step 'I should see new an error that tag ref is invalid' do step 'I should see new an error that tag ref is invalid' do
expect(page).to have_content 'Invalid reference name' expect(page).to have_content 'Target foo is invalid'
end end
step 'I should see new an error that tag already exists' do step 'I should see new an error that tag already exists' do
expect(page).to have_content 'Tag already exists' expect(page).to have_content 'Tag v1.0.0 already exists'
end end
step "I visit tag 'v1.1.0' page" do step "I visit tag 'v1.1.0' page" do
......
...@@ -12,7 +12,7 @@ class Spinach::Features::User < Spinach::FeatureSteps ...@@ -12,7 +12,7 @@ class Spinach::Features::User < Spinach::FeatureSteps
user = User.find_by(name: 'John Doe') user = User.find_by(name: 'John Doe')
project = contributed_project project = contributed_project
# Issue controbution # Issue contribution
issue_params = { title: 'Bug in old browser' } issue_params = { title: 'Bug in old browser' }
Issues::CreateService.new(project, user, issue_params).execute Issues::CreateService.new(project, user, issue_params).execute
...@@ -28,7 +28,7 @@ class Spinach::Features::User < Spinach::FeatureSteps ...@@ -28,7 +28,7 @@ class Spinach::Features::User < Spinach::FeatureSteps
end end
step 'I should see contributed projects' do step 'I should see contributed projects' do
page.within '.contributed-projects' do page.within '#contributed' do
expect(page).to have_content(@contributed_project.name) expect(page).to have_content(@contributed_project.name)
end end
end end
......
...@@ -12,14 +12,20 @@ module API ...@@ -12,14 +12,20 @@ module API
# Parameters: # Parameters:
# id (required) - The ID of a project # id (required) - The ID of a project
# ref_name (optional) - The name of a repository branch or tag, if not given the default branch is used # ref_name (optional) - The name of a repository branch or tag, if not given the default branch is used
# since (optional) - Only commits after or in this date will be returned
# until (optional) - Only commits before or in this date will be returned
# Example Request: # Example Request:
# GET /projects/:id/repository/commits # GET /projects/:id/repository/commits
get ":id/repository/commits" do get ":id/repository/commits" do
datetime_attributes! :since, :until
page = (params[:page] || 0).to_i page = (params[:page] || 0).to_i
per_page = (params[:per_page] || 20).to_i per_page = (params[:per_page] || 20).to_i
ref = params[:ref_name] || user_project.try(:default_branch) || 'master' ref = params[:ref_name] || user_project.try(:default_branch) || 'master'
after = params[:since]
before = params[:until]
commits = user_project.repository.commits(ref, nil, per_page, page * per_page) commits = user_project.repository.commits(ref, limit: per_page, offset: page * per_page, after: after, before: before)
present commits, with: Entities::RepoCommit present commits, with: Entities::RepoCommit
end end
......
...@@ -183,6 +183,22 @@ module API ...@@ -183,6 +183,22 @@ module API
Gitlab::Access.options_with_owner.values.include? level.to_i Gitlab::Access.options_with_owner.values.include? level.to_i
end end
# Checks the occurrences of datetime attributes, each attribute if present in the params hash must be in ISO 8601
# format (YYYY-MM-DDTHH:MM:SSZ) or a Bad Request error is invoked.
#
# Parameters:
# keys (required) - An array consisting of elements that must be parseable as dates from the params hash
def datetime_attributes!(*keys)
keys.each do |key|
begin
params[key] = Time.xmlschema(params[key]) if params[key].present?
rescue ArgumentError
message = "\"" + key.to_s + "\" must be a timestamp in ISO 8601 format: YYYY-MM-DDTHH:MM:SSZ"
render_api_error!(message, 400)
end
end
end
def issuable_order_by def issuable_order_by
if params["order_by"] == 'updated_at' if params["order_by"] == 'updated_at'
'updated_at' 'updated_at'
......
...@@ -79,24 +79,6 @@ module Gitlab ...@@ -79,24 +79,6 @@ module Gitlab
'rm-project', "#{name}.git"]) 'rm-project', "#{name}.git"])
end end
# Add repository tag from passed ref
#
# path - project path with namespace
# tag_name - new tag name
# ref - HEAD for new tag
# message - optional message for tag (annotated tag)
#
# Ex.
# add_tag("gitlab/gitlab-ci", "v4.0", "master")
# add_tag("gitlab/gitlab-ci", "v4.0", "master", "message")
#
def add_tag(path, tag_name, ref, message = nil)
cmd = %W(#{gitlab_shell_path}/bin/gitlab-projects create-tag #{path}.git
#{tag_name} #{ref})
cmd << message unless message.nil? || message.empty?
Gitlab::Utils.system_silent(cmd)
end
# Gc repository # Gc repository
# #
# path - project path with namespace # path - project path with namespace
......
...@@ -66,7 +66,7 @@ module Gitlab ...@@ -66,7 +66,7 @@ module Gitlab
# This method provide a sample data generated with # This method provide a sample data generated with
# existing project and commits to test webhooks # existing project and commits to test webhooks
def build_sample(project, user) def build_sample(project, user)
commits = project.repository.commits(project.default_branch, nil, 3) commits = project.repository.commits(project.default_branch, limit: 3)
ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}" ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}"
build(project, user, commits.last.id, commits.first.id, ref, commits) build(project, user, commits.last.id, commits.first.id, ref, commits)
end end
......
...@@ -37,7 +37,7 @@ feature 'Login', feature: true do ...@@ -37,7 +37,7 @@ feature 'Login', feature: true do
end end
def enter_code(code) def enter_code(code)
fill_in 'Two-factor authentication code', with: code fill_in 'Two-factor Authentication code', with: code
click_button 'Verify code' click_button 'Verify code'
end end
......
...@@ -7,10 +7,10 @@ feature 'Signup', feature: true do ...@@ -7,10 +7,10 @@ feature 'Signup', feature: true do
visit root_path visit root_path
fill_in 'user_name', with: user.name fill_in 'new_user_name', with: user.name
fill_in 'user_username', with: user.username fill_in 'new_user_username', with: user.username
fill_in 'user_email', with: user.email fill_in 'new_user_email', with: user.email
fill_in 'user_password_sign_up', with: user.password fill_in 'new_user_password', with: user.password
click_button "Sign up" click_button "Sign up"
expect(current_path).to eq users_almost_there_path expect(current_path).to eq users_almost_there_path
...@@ -25,10 +25,10 @@ feature 'Signup', feature: true do ...@@ -25,10 +25,10 @@ feature 'Signup', feature: true do
visit root_path visit root_path
fill_in 'user_name', with: user.name fill_in 'new_user_name', with: user.name
fill_in 'user_username', with: user.username fill_in 'new_user_username', with: user.username
fill_in 'user_email', with: existing_user.email fill_in 'new_user_email', with: existing_user.email
fill_in 'user_password_sign_up', with: user.password fill_in 'new_user_password', with: user.password
click_button "Sign up" click_button "Sign up"
expect(current_path).to eq user_registration_path expect(current_path).to eq user_registration_path
...@@ -42,10 +42,10 @@ feature 'Signup', feature: true do ...@@ -42,10 +42,10 @@ feature 'Signup', feature: true do
visit root_path visit root_path
fill_in 'user_name', with: user.name fill_in 'new_user_name', with: user.name
fill_in 'user_username', with: user.username fill_in 'new_user_username', with: user.username
fill_in 'user_email', with: existing_user.email fill_in 'new_user_email', with: existing_user.email
fill_in 'user_password_sign_up', with: user.password fill_in 'new_user_password', with: user.password
click_button "Sign up" click_button "Sign up"
expect(current_path).to eq user_registration_path expect(current_path).to eq user_registration_path
......
...@@ -5,10 +5,10 @@ feature 'Users', feature: true do ...@@ -5,10 +5,10 @@ feature 'Users', feature: true do
scenario 'GET /users/sign_in creates a new user account' do scenario 'GET /users/sign_in creates a new user account' do
visit new_user_session_path visit new_user_session_path
fill_in 'user_name', with: 'Name Surname' fill_in 'new_user_name', with: 'Name Surname'
fill_in 'user_username', with: 'Great' fill_in 'new_user_username', with: 'Great'
fill_in 'user_email', with: 'name@mail.com' fill_in 'new_user_email', with: 'name@mail.com'
fill_in 'user_password_sign_up', with: 'password1234' fill_in 'new_user_password', with: 'password1234'
expect { click_button 'Sign up' }.to change { User.count }.by(1) expect { click_button 'Sign up' }.to change { User.count }.by(1)
end end
...@@ -31,10 +31,10 @@ feature 'Users', feature: true do ...@@ -31,10 +31,10 @@ feature 'Users', feature: true do
scenario 'Should show one error if email is already taken' do scenario 'Should show one error if email is already taken' do
visit new_user_session_path visit new_user_session_path
fill_in 'user_name', with: 'Another user name' fill_in 'new_user_name', with: 'Another user name'
fill_in 'user_username', with: 'anotheruser' fill_in 'new_user_username', with: 'anotheruser'
fill_in 'user_email', with: user.email fill_in 'new_user_email', with: user.email
fill_in 'user_password_sign_up', with: '12341234' fill_in 'new_user_password', with: '12341234'
expect { click_button 'Sign up' }.to change { User.count }.by(0) expect { click_button 'Sign up' }.to change { User.count }.by(0)
expect(page).to have_text('Email has already been taken') expect(page).to have_text('Email has already been taken')
expect(number_of_errors_on_page(page)).to be(1), 'errors on page:\n #{errors_on_page page}' expect(number_of_errors_on_page(page)).to be(1), 'errors on page:\n #{errors_on_page page}'
......
...@@ -131,4 +131,13 @@ describe ProjectsHelper do ...@@ -131,4 +131,13 @@ describe ProjectsHelper do
end end
end end
end end
describe '#sanitized_import_error' do
it 'removes the repo path' do
repo = File.join(Gitlab.config.gitlab_shell.repos_path, '/namespace/test.git')
import_error = "Could not clone #{repo}\n"
expect(sanitize_repo_path(import_error)).to eq('Could not clone [REPOS PATH]/namespace/test.git')
end
end
end end
#= require merge_request_widget
describe 'MergeRequestWidget', ->
beforeEach ->
window.notifyPermissions = () ->
window.notify = () ->
@opts = {
ci_status_url:"http://sampledomain.local/ci/getstatus",
ci_status:"",
ci_message: {
normal: "Build {{status}} for \"{{title}}\"",
preparing: "{{status}} build for \"{{title}}\""
},
ci_title: {
preparing: "{{status}} build",
normal: "Build {{status}}"
},
gitlab_icon:"gitlab_logo.png",
builds_path:"http://sampledomain.local/sampleBuildsPath"
}
@class = new MergeRequestWidget(@opts)
@ciStatusData = {"title":"Sample MR title","sha":"12a34bc5","status":"success","coverage":98}
describe 'getCIStatus', ->
beforeEach ->
spyOn(jQuery, 'getJSON').and.callFake (req, cb) =>
cb(@ciStatusData)
it 'should call showCIStatus even if a notification should not be displayed', ->
spy = spyOn(@class, 'showCIStatus').and.stub()
@class.getCIStatus(false)
expect(spy).toHaveBeenCalledWith(@ciStatusData.status)
it 'should call showCIStatus when a notification should be displayed', ->
spy = spyOn(@class, 'showCIStatus').and.stub()
@class.getCIStatus(true)
expect(spy).toHaveBeenCalledWith(@ciStatusData.status)
it 'should call showCICoverage when the coverage rate is set', ->
spy = spyOn(@class, 'showCICoverage').and.stub()
@class.getCIStatus(false)
expect(spy).toHaveBeenCalledWith(@ciStatusData.coverage)
it 'should not call showCICoverage when the coverage rate is not set', ->
@ciStatusData.coverage = null
spy = spyOn(@class, 'showCICoverage').and.stub()
@class.getCIStatus(false)
expect(spy).not.toHaveBeenCalled()
...@@ -561,7 +561,7 @@ describe Repository, models: true do ...@@ -561,7 +561,7 @@ describe Repository, models: true do
end end
describe :skip_merged_commit do describe :skip_merged_commit do
subject { repository.commits(Gitlab::Git::BRANCH_REF_PREFIX + "'test'", nil, 100, 0, true).map{ |k| k.id } } subject { repository.commits(Gitlab::Git::BRANCH_REF_PREFIX + "'test'", limit: 100, skip_merges: true).map{ |k| k.id } }
it { is_expected.not_to include('e56497bb5f03a90a51293fc6d516788730953899') } it { is_expected.not_to include('e56497bb5f03a90a51293fc6d516788730953899') }
end end
...@@ -858,13 +858,30 @@ describe Repository, models: true do ...@@ -858,13 +858,30 @@ describe Repository, models: true do
end end
describe '#add_tag' do describe '#add_tag' do
it 'adds a tag' do context 'with a valid target' do
expect(repository).to receive(:before_push_tag) let(:user) { build_stubbed(:user) }
expect_any_instance_of(Gitlab::Shell).to receive(:add_tag). it 'creates the tag using rugged' do
with(repository.path_with_namespace, '8.5', 'master', 'foo') expect(repository.rugged.tags).to receive(:create).
with('8.5', repository.commit('master').id,
hash_including(message: 'foo',
tagger: hash_including(name: user.name, email: user.email))).
and_call_original
repository.add_tag('8.5', 'master', 'foo') repository.add_tag(user, '8.5', 'master', 'foo')
end
it 'returns a Gitlab::Git::Tag object' do
tag = repository.add_tag(user, '8.5', 'master', 'foo')
expect(tag).to be_a(Gitlab::Git::Tag)
end
end
context 'with an invalid target' do
it 'returns false' do
expect(repository.add_tag(user, '8.5', 'bar', 'foo')).to be false
end
end end
end end
......
...@@ -32,6 +32,41 @@ describe API::API, api: true do ...@@ -32,6 +32,41 @@ describe API::API, api: true do
expect(response.status).to eq(401) expect(response.status).to eq(401)
end end
end end
context "since optional parameter" do
it "should return project commits since provided parameter" do
commits = project.repository.commits("master")
since = commits.second.created_at
get api("/projects/#{project.id}/repository/commits?since=#{since.utc.iso8601}", user)
expect(json_response.size).to eq 2
expect(json_response.first["id"]).to eq(commits.first.id)
expect(json_response.second["id"]).to eq(commits.second.id)
end
end
context "until optional parameter" do
it "should return project commits until provided parameter" do
commits = project.repository.commits("master")
before = commits.second.created_at
get api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user)
expect(json_response.size).to eq(commits.size - 1)
expect(json_response.first["id"]).to eq(commits.second.id)
expect(json_response.second["id"]).to eq(commits.third.id)
end
end
context "invalid xmlschema date parameters" do
it "should return an invalid parameter error message" do
get api("/projects/#{project.id}/repository/commits?since=invalid-date", user)
expect(response.status).to eq(400)
expect(json_response['message']).to include "\"since\" must be a timestamp in ISO 8601 format"
end
end
end end
describe "GET /projects:id/repository/commits/:sha" do describe "GET /projects:id/repository/commits/:sha" do
......
...@@ -147,7 +147,7 @@ describe API::API, api: true do ...@@ -147,7 +147,7 @@ describe API::API, api: true do
tag_name: 'v8.0.0', tag_name: 'v8.0.0',
ref: 'master' ref: 'master'
expect(response.status).to eq(400) expect(response.status).to eq(400)
expect(json_response['message']).to eq('Tag already exists') expect(json_response['message']).to eq('Tag v8.0.0 already exists')
end end
it 'should return 400 if ref name is invalid' do it 'should return 400 if ref name is invalid' do
...@@ -155,7 +155,7 @@ describe API::API, api: true do ...@@ -155,7 +155,7 @@ describe API::API, api: true do
tag_name: 'mytag', tag_name: 'mytag',
ref: 'foo' ref: 'foo'
expect(response.status).to eq(400) expect(response.status).to eq(400)
expect(json_response['message']).to eq('Invalid reference name') expect(json_response['message']).to eq('Target foo is invalid')
end end
end end
......
require 'spec_helper'
describe CreateTagService, services: true do
let(:project) { create(:project) }
let(:repository) { project.repository }
let(:user) { create(:user) }
let(:service) { described_class.new(project, user) }
describe '#execute' do
it 'creates the tag and returns success' do
response = service.execute('v42.42.42', 'master', 'Foo')
expect(response[:status]).to eq(:success)
expect(response[:tag]).to be_a Gitlab::Git::Tag
expect(response[:tag].name).to eq('v42.42.42')
end
context 'when target is invalid' do
it 'returns an error' do
response = service.execute('v1.1.0', 'foo', 'Foo')
expect(response).to eq(status: :error,
message: 'Target foo is invalid')
end
end
context 'when tag already exists' do
it 'returns an error' do
expect(repository).to receive(:add_tag).
with(user, 'v1.1.0', 'master', 'Foo').
and_raise(Rugged::TagError)
response = service.execute('v1.1.0', 'master', 'Foo')
expect(response).to eq(status: :error,
message: 'Tag v1.1.0 already exists')
end
end
context 'when pre-receive hook fails' do
it 'returns an error' do
expect(repository).to receive(:add_tag).
with(user, 'v1.1.0', 'master', 'Foo').
and_raise(GitHooksService::PreReceiveError)
response = service.execute('v1.1.0', 'master', 'Foo')
expect(response).to eq(status: :error,
message: 'Tag creation was rejected by Git hook')
end
end
end
end
...@@ -51,10 +51,4 @@ FactoryGirl::SyntaxRunner.class_eval do ...@@ -51,10 +51,4 @@ FactoryGirl::SyntaxRunner.class_eval do
include RSpec::Mocks::ExampleMethods include RSpec::Mocks::ExampleMethods
end end
# Work around a Rails 4.2.5.1 issue
# See https://github.com/rspec/rspec-rails/issues/1532
RSpec::Rails::ViewRendering::EmptyTemplatePathSetDecorator.class_eval do
alias_method :find_all_anywhere, :find_all
end
ActiveRecord::Migration.maintain_test_schema! ActiveRecord::Migration.maintain_test_schema!
...@@ -12,7 +12,7 @@ describe RepositoryCheck::SingleRepositoryWorker do ...@@ -12,7 +12,7 @@ describe RepositoryCheck::SingleRepositoryWorker do
subject.perform(project.id) subject.perform(project.id)
expect(project.reload.last_repository_check_failed).to eq(false) expect(project.reload.last_repository_check_failed).to eq(false)
destroy_wiki(project) break_wiki(project)
subject.perform(project.id) subject.perform(project.id)
expect(project.reload.last_repository_check_failed).to eq(true) expect(project.reload.last_repository_check_failed).to eq(true)
...@@ -20,15 +20,38 @@ describe RepositoryCheck::SingleRepositoryWorker do ...@@ -20,15 +20,38 @@ describe RepositoryCheck::SingleRepositoryWorker do
it 'skips wikis when disabled' do it 'skips wikis when disabled' do
project = create(:project_empty_repo, wiki_enabled: false) project = create(:project_empty_repo, wiki_enabled: false)
# Make sure the test would fail if it checked the wiki repo # Make sure the test would fail if the wiki repo was checked
destroy_wiki(project) break_wiki(project)
subject.perform(project.id) subject.perform(project.id)
expect(project.reload.last_repository_check_failed).to eq(false) expect(project.reload.last_repository_check_failed).to eq(false)
end end
def destroy_wiki(project) it 'creates missing wikis' do
FileUtils.rm_rf(project.wiki.repository.path_to_repo) project = create(:project_empty_repo, wiki_enabled: true)
FileUtils.rm_rf(wiki_path(project))
subject.perform(project.id)
expect(project.reload.last_repository_check_failed).to eq(false)
end
it 'does not create a wiki if the main repo does not exist at all' do
project = create(:project_empty_repo)
FileUtils.rm_rf(project.repository.path_to_repo)
FileUtils.rm_rf(wiki_path(project))
subject.perform(project.id)
expect(File.exist?(wiki_path(project))).to eq(false)
end
def break_wiki(project)
FileUtils.rm_rf(wiki_path(project) + '/objects')
end
def wiki_path(project)
project.wiki.repository.path_to_repo
end end
end end
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