diff options
92 files changed, 1716 insertions, 468 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 9411cc62003..7ed09fb1db8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ entry. - Show correct environment log in admin/logs (@duk3luk3 !7191) - Fix Milestone dropdown not stay selected for `Upcoming` and `No Milestone` option !7117 +- Diff collapse won't shift when collapsing. - Backups do not fail anymore when using tar on annex and custom_hooks only. !5814 - Adds user project membership expired event to clarify why user was removed (Callum Dryden) - Trim leading and trailing whitespace on project_path (Linus Thiel) @@ -13,6 +14,7 @@ entry. - Adds support for the `token` attribute in project hooks API (Gauvain Pocentek) - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO) - Fix Markdown styling inside reference links (Jan Zdráhal) +- Create new issue board list after creating a new label - Fix extra space on Build sidebar on Firefox !7060 - Fail gracefully when creating merge request with non-existing branch (alexsanford) - Fix mobile layout issues in admin user overview page !7087 @@ -66,6 +68,7 @@ entry. - In all filterable drop downs, put input field in focus only after load is complete (Ido @leibo) - Improve search query parameter naming in /admin/users !7115 (YarNayar) - Fix table pagination to be responsive +- Fix applying GitHub-imported labels when importing job is interrupted - Allow to search for user by secondary email address in the admin interface(/admin/users) !7115 (YarNayar) - Updated commit SHA styling on the branches page. diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 7ada0d303f3..3eefcb9dd5b 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -0.8.5 +1.0.0 diff --git a/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 b/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 index fe1a6dc7ea0..14f618fd5d5 100644 --- a/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 +++ b/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 @@ -2,6 +2,19 @@ $(() => { const Store = gl.issueBoards.BoardsStore; + $(document).off('created.label').on('created.label', (e, label) => { + Store.new({ + title: label.title, + position: Store.state.lists.length - 2, + list_type: 'label', + label: { + id: label.id, + title: label.title, + color: label.color + } + }); + }); + $('.js-new-board-list').each(function () { const $this = $(this); new gl.CreateLabelDropdown($this.closest('.dropdown').find('.dropdown-new-label'), $this.data('namespace-path'), $this.data('project-path')); diff --git a/app/assets/javascripts/create_label.js.es6 b/app/assets/javascripts/create_label.js.es6 index f20580b1279..744aa0afa03 100644 --- a/app/assets/javascripts/create_label.js.es6 +++ b/app/assets/javascripts/create_label.js.es6 @@ -115,6 +115,8 @@ .show(); } else { this.$dropdownBack.trigger('click'); + + $(document).trigger('created.label', label); } }); } diff --git a/app/assets/javascripts/extensions/element.js.es6 b/app/assets/javascripts/extensions/element.js.es6 index c74fc9ad074..afb2f0d6956 100644 --- a/app/assets/javascripts/extensions/element.js.es6 +++ b/app/assets/javascripts/extensions/element.js.es6 @@ -1,5 +1,7 @@ -/* eslint-disable */ -Element.prototype.matches = Element.prototype.matches || Element.prototype.msMatches; +/* global Element */ +/* eslint-disable consistent-return, max-len */ + +Element.prototype.matches = Element.prototype.matches || Element.prototype.msMatchesSelector; Element.prototype.closest = function closest(selector, selectedElement = this) { if (!selectedElement) return; diff --git a/app/assets/stylesheets/framework/avatar.scss b/app/assets/stylesheets/framework/avatar.scss index ce117c3fba5..202ed5ae8fe 100644 --- a/app/assets/stylesheets/framework/avatar.scss +++ b/app/assets/stylesheets/framework/avatar.scss @@ -4,7 +4,7 @@ margin-right: $margin-right; } -.avatar-container { +.avatar-circle { float: left; margin-right: 15px; border-radius: $avatar_radius; @@ -27,7 +27,7 @@ } .avatar { - @extend .avatar-container; + @extend .avatar-circle; width: 40px; height: 40px; padding: 0; @@ -64,8 +64,8 @@ &.s160 { font-size: 96px; line-height: 158px; } } -.image-container { - @extend .avatar-container; +.avatar-container { + @extend .avatar-circle; overflow: hidden; display: flex; @@ -76,4 +76,4 @@ margin: 0; align-self: center; } -}
\ No newline at end of file +} diff --git a/app/assets/stylesheets/pages/commit.scss b/app/assets/stylesheets/pages/commit.scss index 8ecf7fcb96d..47d3e72679b 100644 --- a/app/assets/stylesheets/pages/commit.scss +++ b/app/assets/stylesheets/pages/commit.scss @@ -36,9 +36,42 @@ padding: 10px 0; margin-bottom: 0; - .commit-options-dropdown-caret { - @media (max-width: $screen-sm) { - margin-left: 0; + @media (min-width: $screen-sm-min) { + display: flex; + align-items: center; + + .commit-meta { + flex: 1; + } + } + + .commit-hash-full { + @media (max-width: $screen-sm-max) { + width: 80px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + display: inline-block; + vertical-align: bottom; + } + } + + .commit-action-buttons { + i { + color: $gl-icon-color; + font-size: 13px; + margin-right: 3px; + } + + @media (max-width: $screen-xs-max) { + .dropdown { + width: 100%; + margin-top: 10px; + } + + .dropdown-toggle { + width: 100%; + } } } } @@ -188,17 +221,6 @@ } } -.commit-action-buttons { - position: relative; - top: -1px; - - i { - color: $gl-icon-color; - font-size: 13px; - margin-right: 3px; - } -} - /* * Commit message textarea for web editor and * custom merge request message diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 86e808314f4..52e0256943a 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -117,6 +117,11 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :send_user_confirmation_email, :container_registry_token_expire_delay, :enabled_git_access_protocol, + :housekeeping_enabled, + :housekeeping_bitmaps_enabled, + :housekeeping_incremental_repack_period, + :housekeeping_full_repack_period, + :housekeeping_gc_period, repository_storages: [], restricted_visibility_levels: [], import_sources: [], diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index e13b7cdd707..07ff6fb9488 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -179,33 +179,6 @@ module BlobHelper } end - def selected_template(issuable) - templates = issuable_templates(issuable) - params[:issuable_template] if templates.include?(params[:issuable_template]) - end - - def can_add_template?(issuable) - names = issuable_templates(issuable) - names.empty? && can?(current_user, :push_code, @project) && !@project.private? - end - - def merge_request_template_names - @merge_request_templates ||= Gitlab::Template::MergeRequestTemplate.dropdown_names(ref_project) - end - - def issue_template_names - @issue_templates ||= Gitlab::Template::IssueTemplate.dropdown_names(ref_project) - end - - def issuable_templates(issuable) - @issuable_templates ||= - if issuable.is_a?(Issue) - issue_template_names - elsif issuable.is_a?(MergeRequest) - merge_request_template_names - end - end - def ref_project @ref_project ||= @target_project || @project end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index ef6cfb235a9..8127c3f3ee3 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -30,6 +30,33 @@ module IssuablesHelper end end + def can_add_template?(issuable) + names = issuable_templates(issuable) + names.empty? && can?(current_user, :push_code, @project) && !@project.private? + end + + def template_dropdown_tag(issuable, &block) + title = selected_template(issuable) || "Choose a template" + options = { + toggle_class: 'js-issuable-selector', + title: title, + filter: true, + placeholder: 'Filter', + footer_content: true, + data: { + data: issuable_templates(issuable), + field_name: 'issuable_template', + selected: selected_template(issuable), + project_path: ref_project.path, + namespace_path: ref_project.namespace.path + } + } + + dropdown_tag(title, options: options) do + capture(&block) + end + end + def user_dropdown_label(user_id, default_label) return default_label if user_id.nil? return "Unassigned" if user_id == "0" @@ -153,4 +180,28 @@ module IssuablesHelper hexdigest(['issuables_count', issuable_type, opts.sort].flatten.join('-')) end + + def issuable_templates(issuable) + @issuable_templates ||= + case issuable + when Issue + issue_template_names + when MergeRequest + merge_request_template_names + else + raise 'Unknown issuable type!' + end + end + + def merge_request_template_names + @merge_request_templates ||= Gitlab::Template::MergeRequestTemplate.dropdown_names(ref_project) + end + + def issue_template_names + @issue_templates ||= Gitlab::Template::IssueTemplate.dropdown_names(ref_project) + end + + def selected_template(issuable) + params[:issuable_template] if issuable_templates(issuable).include?(params[:issuable_template]) + end end diff --git a/app/mailers/base_mailer.rb b/app/mailers/base_mailer.rb index 61a574d3dc0..79c3c2e62c5 100644 --- a/app/mailers/base_mailer.rb +++ b/app/mailers/base_mailer.rb @@ -1,6 +1,6 @@ class BaseMailer < ActionMailer::Base - add_template_helper ApplicationHelper - add_template_helper GitlabMarkdownHelper + helper ApplicationHelper + helper GitlabMarkdownHelper attr_accessor :current_user helper_method :current_user, :can? diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index eca6ec29767..0bc1c19e9cd 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -10,12 +10,12 @@ class Notify < BaseMailer include Emails::Pipelines include Emails::Members - add_template_helper MergeRequestsHelper - add_template_helper DiffHelper - add_template_helper BlobHelper - add_template_helper EmailsHelper - add_template_helper MembersHelper - add_template_helper GitlabRoutingHelper + helper MergeRequestsHelper + helper DiffHelper + helper BlobHelper + helper EmailsHelper + helper MembersHelper + helper GitlabRoutingHelper def test_email(recipient_email, subject, body) mail(to: recipient_email, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 6e7a90e7d9c..bb60cc8736c 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -85,6 +85,18 @@ class ApplicationSetting < ActiveRecord::Base presence: { message: 'Domain blacklist cannot be empty if Blacklist is enabled.' }, if: :domain_blacklist_enabled? + validates :housekeeping_incremental_repack_period, + presence: true, + numericality: { only_integer: true, greater_than: 0 } + + validates :housekeeping_full_repack_period, + presence: true, + numericality: { only_integer: true, greater_than: :housekeeping_incremental_repack_period } + + validates :housekeeping_gc_period, + presence: true, + numericality: { only_integer: true, greater_than: :housekeeping_full_repack_period } + validates_each :restricted_visibility_levels do |record, attr, value| unless value.nil? value.each do |level| @@ -168,6 +180,11 @@ class ApplicationSetting < ActiveRecord::Base container_registry_token_expire_delay: 5, repository_storages: ['default'], user_default_external: false, + housekeeping_enabled: true, + housekeeping_bitmaps_enabled: true, + housekeeping_incremental_repack_period: 10, + housekeeping_full_repack_period: 50, + housekeeping_gc_period: 200, ) end @@ -202,11 +219,7 @@ class ApplicationSetting < ActiveRecord::Base end def repository_storages - value = read_attribute(:repository_storages) - value = [value] if value.is_a?(String) - value = [] if value.nil? - - value + Array(read_attribute(:repository_storages)) end # repository_storage is still required in the API. Remove in 9.0 diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 613444e0d70..93a6b3122e0 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -286,6 +286,11 @@ module Issuable false end + def assignee_or_author?(user) + # We're comparing IDs here so we don't need to load any associations. + author_id == user.id || assignee_id == user.id + end + def record_metrics metrics = self.metrics || create_metrics metrics.record! diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb index fd9a8c1b8b7..91b508eb325 100644 --- a/app/models/external_issue.rb +++ b/app/models/external_issue.rb @@ -29,6 +29,15 @@ class ExternalIssue @project end + def project_id + @project.id + end + + # Pattern used to extract `JIRA-123` issue references from text + def self.reference_pattern + @reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)} + end + def to_reference(_from_project = nil) id end diff --git a/app/models/issue_collection.rb b/app/models/issue_collection.rb new file mode 100644 index 00000000000..f0b7d9914c8 --- /dev/null +++ b/app/models/issue_collection.rb @@ -0,0 +1,42 @@ +# IssueCollection can be used to reduce a list of issues down to a subset. +# +# IssueCollection is not meant to be some sort of Enumerable, instead it's meant +# to take a list of issues and return a new list of issues based on some +# criteria. For example, given a list of issues you may want to return a list of +# issues that can be read or updated by a given user. +class IssueCollection + attr_reader :collection + + def initialize(collection) + @collection = collection + end + + # Returns all the issues that can be updated by the user. + def updatable_by_user(user) + return collection if user.admin? + + # Given all the issue projects we get a list of projects that the current + # user has at least reporter access to. + projects_with_reporter_access = user. + projects_with_reporter_access_limited_to(project_ids). + pluck(:id) + + collection.select do |issue| + if projects_with_reporter_access.include?(issue.project_id) + true + elsif issue.is_a?(Issue) + issue.assignee_or_author?(user) + else + false + end + end + end + + alias_method :visible_to, :updatable_by_user + + private + + def project_ids + @project_ids ||= collection.map(&:project_id).uniq + end +end diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 0a493b7a12b..2dbe0075465 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -163,6 +163,21 @@ class JiraService < IssueTrackerService add_comment(data, issue_key) end + # reason why service cannot be tested + def disabled_title + "Please fill in Password and Username." + end + + def can_test? + username.present? && password.present? + end + + # JIRA does not need test data. + # We are requesting the project that belongs to the project key. + def test_data(user = nil, project = nil) + nil + end + def test_settings return unless url.present? # Test settings by getting the project diff --git a/app/models/user.rb b/app/models/user.rb index 65e96ee6b2e..c0dffa7b6ea 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -444,6 +444,16 @@ class User < ActiveRecord::Base Project.where("projects.id IN (#{projects_union(min_access_level).to_sql})") end + # Returns the projects this user has reporter (or greater) access to, limited + # to at most the given projects. + # + # This method is useful when you have a list of projects and want to + # efficiently check to which of these projects the user has at least reporter + # access. + def projects_with_reporter_access_limited_to(projects) + authorized_projects(Gitlab::Access::REPORTER).where(id: projects) + end + def viewable_starred_projects starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (#{projects_union.to_sql})", [Project::PUBLIC, Project::INTERNAL]) diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb index c253f9a9399..9501e499507 100644 --- a/app/policies/issuable_policy.rb +++ b/app/policies/issuable_policy.rb @@ -4,7 +4,7 @@ class IssuablePolicy < BasePolicy end def rules - if @user && (@subject.author == @user || @subject.assignee == @user) + if @user && @subject.assignee_or_author?(@user) can! :"read_#{action_name}" can! :"update_#{action_name}" end diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb index bd1811a3c54..52fa33bc4b0 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -8,9 +8,8 @@ class IssuePolicy < IssuablePolicy if @subject.confidential? && !can_read_confidential? cannot! :read_issue - cannot! :admin_issue cannot! :update_issue - cannot! :read_issue + cannot! :admin_issue end end @@ -18,11 +17,7 @@ class IssuePolicy < IssuablePolicy def can_read_confidential? return false unless @user - return true if @user.admin? - return true if @subject.author == @user - return true if @subject.assignee == @user - return true if @subject.project.team.member?(@user, Gitlab::Access::REPORTER) - false + IssueCollection.new([@subject]).visible_to(@user).any? end end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index e8415862de5..de313095bed 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -105,35 +105,11 @@ class GitPushService < BaseService # Extract any GFM references from the pushed commit messages. If the configured issue-closing regex is matched, # close the referenced Issue. Create cross-reference Notes corresponding to any other referenced Mentionables. def process_commit_messages - is_default_branch = is_default_branch? - - authors = Hash.new do |hash, commit| - email = commit.author_email - next hash[email] if hash.has_key?(email) - - hash[email] = commit_user(commit) - end + default = is_default_branch? @push_commits.each do |commit| - # Keep track of the issues that will be actually closed because they are on a default branch. - # Hence, when creating cross-reference notes, the not-closed issues (on non-default branches) - # will also have cross-reference. - closed_issues = [] - - if is_default_branch - # Close issues if these commits were pushed to the project's default branch and the commit message matches the - # closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to - # a different branch. - closed_issues = commit.closes_issues(current_user) - closed_issues.each do |issue| - if can?(current_user, :update_issue, issue) - Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit: commit) - end - end - end - - commit.create_cross_references!(authors[commit], closed_issues) - update_issue_metrics(commit, authors) + ProcessCommitWorker. + perform_async(project.id, current_user.id, commit.id, default) end end @@ -176,11 +152,4 @@ class GitPushService < BaseService def branch_name @branch_name ||= Gitlab::Git.ref_name(params[:ref]) end - - def update_issue_metrics(commit, authors) - mentioned_issues = commit.all_references(authors[commit]).issues - - Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil). - update_all(first_mentioned_in_commit_at: commit.committed_date) - end end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 45cca216ccc..ab4c51386a4 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -1,8 +1,21 @@ module Issues class CloseService < Issues::BaseService + # Closes the supplied issue if the current user is able to do so. def execute(issue, commit: nil, notifications: true, system_note: true) return issue unless can?(current_user, :update_issue, issue) + close_issue(issue, + commit: commit, + notifications: notifications, + system_note: system_note) + end + + # Closes the supplied issue without checking if the user is authorized to + # do so. + # + # The code calling this method is responsible for ensuring that a user is + # allowed to close the given issue. + def close_issue(issue, commit: nil, notifications: true, system_note: true) if project.jira_tracker? && project.jira_service.active project.jira_service.execute(commit, issue) todo_service.close_issue(issue, current_user) diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb index c3dfc8cfbe8..4b8946f8ee2 100644 --- a/app/services/projects/housekeeping_service.rb +++ b/app/services/projects/housekeeping_service.rb @@ -7,6 +7,8 @@ # module Projects class HousekeepingService < BaseService + include Gitlab::CurrentSettings + LEASE_TIMEOUT = 3600 class LeaseTaken < StandardError @@ -20,13 +22,14 @@ module Projects end def execute - raise LeaseTaken unless try_obtain_lease + lease_uuid = try_obtain_lease + raise LeaseTaken unless lease_uuid.present? - execute_gitlab_shell_gc + execute_gitlab_shell_gc(lease_uuid) end def needed? - @project.pushes_since_gc >= 10 + pushes_since_gc > 0 && period_match? && housekeeping_enabled? end def increment! @@ -37,19 +40,59 @@ module Projects private - def execute_gitlab_shell_gc - GitGarbageCollectWorker.perform_async(@project.id) + def execute_gitlab_shell_gc(lease_uuid) + GitGarbageCollectWorker.perform_async(@project.id, task, lease_key, lease_uuid) ensure - Gitlab::Metrics.measure(:reset_pushes_since_gc) do - @project.reset_pushes_since_gc + if pushes_since_gc >= gc_period + Gitlab::Metrics.measure(:reset_pushes_since_gc) do + @project.reset_pushes_since_gc + end end end def try_obtain_lease Gitlab::Metrics.measure(:obtain_housekeeping_lease) do - lease = ::Gitlab::ExclusiveLease.new("project_housekeeping:#{@project.id}", timeout: LEASE_TIMEOUT) + lease = ::Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT) lease.try_obtain end end + + def lease_key + "project_housekeeping:#{@project.id}" + end + + def pushes_since_gc + @project.pushes_since_gc + end + + def task + if pushes_since_gc % gc_period == 0 + :gc + elsif pushes_since_gc % full_repack_period == 0 + :full_repack + else + :incremental_repack + end + end + + def period_match? + [gc_period, full_repack_period, repack_period].any? { |period| pushes_since_gc % period == 0 } + end + + def housekeeping_enabled? + current_application_settings.housekeeping_enabled + end + + def gc_period + current_application_settings.housekeeping_gc_period + end + + def full_repack_period + current_application_settings.housekeeping_full_repack_period + end + + def repack_period + current_application_settings.housekeeping_incremental_repack_period + end end end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 28003e5f509..450ec322f2c 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -422,5 +422,44 @@ Enable this option to include the name of the author of the issue, merge request or comment in the email body instead. + %fieldset + %legend Automatic Git repository housekeeping + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :housekeeping_enabled do + = f.check_box :housekeeping_enabled + Enable automatic repository housekeeping (git repack, git gc) + .help-block + If you keep automatic housekeeping disabled for a long time Git + repository access on your GitLab server will become slower and your + repositories will use more disk space. We recommend to always leave + this enabled. + .checkbox + = f.label :housekeeping_bitmaps_enabled do + = f.check_box :housekeeping_bitmaps_enabled + Enable Git pack file bitmap creation + .help-block + Creating pack file bitmaps makes housekeeping take a little longer but + bitmaps should accelerate 'git clone' performance. + .form-group + = f.label :housekeeping_incremental_repack_period, 'Incremental repack period', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :housekeeping_incremental_repack_period, class: 'form-control' + .help-block + Number of Git pushes after which an incremental 'git repack' is run. + .form-group + = f.label :housekeeping_full_repack_period, 'Full repack period', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :housekeeping_full_repack_period, class: 'form-control' + .help-block + Number of Git pushes after which a full 'git repack' is run. + .form-group + = f.label :housekeeping_gc_period, 'Git GC period', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :housekeeping_gc_period, class: 'form-control' + .help-block + Number of Git pushes after which 'git gc' is run. + .form-actions = f.submit 'Save', class: 'btn btn-save' diff --git a/app/views/admin/groups/_group.html.haml b/app/views/admin/groups/_group.html.haml index 05c88ca1cc8..664bb417c6a 100644 --- a/app/views/admin/groups/_group.html.haml +++ b/app/views/admin/groups/_group.html.haml @@ -16,7 +16,7 @@ %span.visibility-icon.has-tooltip{data: { container: 'body', placement: 'left' }, title: visibility_icon_description(group)} = visibility_level_icon(group.visibility_level, fw: false) - .image-container.s40 + .avatar-container.s40 = image_tag group_icon(group), class: "avatar s40 hidden-xs" .title = link_to [:admin, group], class: 'group-name' do diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index a7c1a4f5038..40871e32913 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -13,7 +13,7 @@ Group info: %ul.well-list %li - .image-container.s60 + .avatar-container.s60 = image_tag group_icon(@group), class: "avatar s60" %li %span.light Name: diff --git a/app/views/admin/projects/index.html.haml b/app/views/admin/projects/index.html.haml index 10dce6f3d8f..b37b8d4fee7 100644 --- a/app/views/admin/projects/index.html.haml +++ b/app/views/admin/projects/index.html.haml @@ -76,7 +76,7 @@ .title = link_to [:admin, project.namespace.becomes(Namespace), project] do .dash-project-avatar - .image-container.s40 + .avatar-container.s40 = project_icon(project, alt: '', class: 'avatar project-avatar s40') %span.project-full-name %span.namespace-name diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index 2f90c19d4b4..2706e8692d1 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -8,7 +8,7 @@ .form-group .col-sm-offset-2.col-sm-10 - .image-container.s160 + .avatar-container.s160 = image_tag group_icon(@group), alt: '', class: 'avatar group-avatar s160' %p.light - if @group.avatar? diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 275581b3af8..b439b40a75a 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -6,7 +6,7 @@ .cover-block.groups-cover-block %div{ class: container_class } - .image-container.s70.group-avatar + .avatar-container.s70.group-avatar = image_tag group_icon(@group), class: "avatar s70 avatar-tile" .group-info .cover-title diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index e67b66d1fff..5a04c3318cf 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -1,7 +1,7 @@ - empty_repo = @project.empty_repo? .project-home-panel.text-center{ class: ("empty-project" if empty_repo) } %div{ class: container_class } - .image-container.s70.project-avatar + .avatar-container.s70.project-avatar = project_icon(@project, alt: @project.name, class: 'avatar s70 avatar-tile') %h1.project-title = @project.name diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index d8c95376b94..0ebc38d16cf 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -1,25 +1,25 @@ .commit-info-row.commit-info-row-header - %span.hidden-xs.hidden-sm Commit - = link_to @commit.short_id, namespace_project_commit_path(@project.namespace, @project, @commit), class: "monospace js-details-short" - = link_to("#", class: "js-details-expand hidden-xs hidden-sm") do - %span.text-expander - \... - %span.js-details-content.hide - = link_to @commit.id, namespace_project_commit_path(@project.namespace, @project, @commit), class: "monospace hidden-xs hidden-sm" - = clipboard_button(clipboard_text: @commit.id) - %span.hidden-xs authored - #{time_ago_with_tooltip(@commit.authored_date)} - %span by - = author_avatar(@commit, size: 24) - %strong - = commit_author_link(@commit, avatar: true, size: 24) - - if @commit.different_committer? - %span.light Committed by + .commit-meta + %strong Commit + %strong.monospace.js-details-short= @commit.short_id + = link_to("#", class: "js-details-expand hidden-xs hidden-sm") do + %span.text-expander + \... + %span.js-details-content.hide + %strong.monospace.commit-hash-full= @commit.id + = clipboard_button(clipboard_text: @commit.id) + %span.hidden-xs authored + #{time_ago_with_tooltip(@commit.authored_date)} + %span by + = author_avatar(@commit, size: 24) %strong - = commit_committer_link(@commit, avatar: true, size: 24) - #{time_ago_with_tooltip(@commit.committed_date)} - - .pull-right.commit-action-buttons + = commit_author_link(@commit, avatar: true, size: 24) + - if @commit.different_committer? + %span.light Committed by + %strong + = commit_committer_link(@commit, avatar: true, size: 24) + #{time_ago_with_tooltip(@commit.committed_date)} + .commit-action-buttons - if defined?(@notes_count) && @notes_count > 0 %span.btn.disabled.btn-grouped.hidden-xs.append-right-10 = icon('comment') @@ -28,8 +28,8 @@ Browse Files .dropdown.inline %a.btn.btn-default.dropdown-toggle{ data: { toggle: "dropdown" } } - %span.hidden-xs Options - = icon('caret-down', class: ".commit-options-dropdown-caret") + %span Options + = icon('caret-down') %ul.dropdown-menu.dropdown-menu-align-right %li.visible-xs-block.visible-sm-block = link_to namespace_project_tree_path(@project.namespace, @project, @commit) do diff --git a/app/views/projects/diffs/_file_header.html.haml b/app/views/projects/diffs/_file_header.html.haml index 73993f35b39..d3ed8e1bf38 100644 --- a/app/views/projects/diffs/_file_header.html.haml +++ b/app/views/projects/diffs/_file_header.html.haml @@ -1,4 +1,4 @@ -%i.fa.diff-toggle-caret +%i.fa.diff-toggle-caret.fa-fw - if defined?(blob) && blob && diff_file.submodule? %span = icon('archive fw') diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index a5422966617..0aa8801c2d8 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -118,7 +118,7 @@ Project avatar .form-group - if @project.avatar? - .image-container.s160 + .avatar-container.s160 = project_icon("#{@project.namespace.to_param}/#{@project.to_param}", alt: '', class: 'avatar project-avatar s160') %p.light - if @project.avatar_in_git diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index 752fbc21a11..b41edeb2c7e 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -12,6 +12,9 @@ = form.submit 'Save changes', class: 'btn btn-save' - if @service.valid? && @service.activated? - - disabled = @service.can_test? ? '':'disabled' - = link_to 'Test settings', test_namespace_project_service_path(@project.namespace, @project, @service), class: "btn #{disabled}", title: @service.disabled_title + - unless @service.can_test? + - disabled_class = 'disabled' + - disabled_title = @service.disabled_title + + = link_to 'Test settings', test_namespace_project_service_path(@project.namespace, @project, @service), class: "btn #{disabled_class}", title: disabled_title = link_to "Cancel", namespace_project_services_path(@project.namespace, @project), class: "btn btn-cancel" diff --git a/app/views/shared/groups/_group.html.haml b/app/views/shared/groups/_group.html.haml index 562291a61df..19221e3391f 100644 --- a/app/views/shared/groups/_group.html.haml +++ b/app/views/shared/groups/_group.html.haml @@ -24,7 +24,7 @@ %span.visibility-icon.has-tooltip{data: { container: 'body', placement: 'left' }, title: visibility_icon_description(group)} = visibility_level_icon(group.visibility_level, fw: false) - .image-container.s40 + .avatar-container.s40 = image_tag group_icon(group), class: "avatar s40 hidden-xs" .title = link_to group, class: 'group-name' do diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 0ace6be8f4e..8d976952781 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -1,4 +1,5 @@ - project = @target_project || @project + = form_errors(issuable) - if @conflict @@ -11,23 +12,9 @@ .form-group = f.label :title, class: 'control-label' - - issuable_template_names = issuable_templates(issuable) - - - if issuable_template_names.any? - .col-sm-3.col-lg-2 - .js-issuable-selector-wrap{ data: { issuable_type: issuable.class.to_s.underscore.downcase } } - - title = selected_template(issuable) || "Choose a template" - - = dropdown_tag(title, options: { toggle_class: 'js-issuable-selector', - title: title, filter: true, placeholder: 'Filter', footer_content: true, - data: { data: issuable_template_names, field_name: 'issuable_template', selected: selected_template(issuable), project_path: ref_project.path, namespace_path: ref_project.namespace.path } } ) do - %ul.dropdown-footer-list - %li - %a.no-template - No template - %a.reset-template - Reset template - %div{ class: issuable_template_names.any? ? 'col-sm-7 col-lg-8' : 'col-sm-10' } + = render 'shared/issuable/form/template_selector', issuable: issuable + + %div{ class: issuable_templates(issuable).any? ? 'col-sm-7 col-lg-8' : 'col-sm-10' } = f.text_field :title, maxlength: 255, autofocus: true, autocomplete: 'off', class: 'form-control pad', required: true diff --git a/app/views/shared/issuable/form/_template_selector.html.haml b/app/views/shared/issuable/form/_template_selector.html.haml new file mode 100644 index 00000000000..d613bd31d81 --- /dev/null +++ b/app/views/shared/issuable/form/_template_selector.html.haml @@ -0,0 +1,13 @@ +- issuable = local_assigns.fetch(:issuable, nil) + +- return unless issuable && issuable_templates(issuable).any? + +.col-sm-3.col-lg-2 + .js-issuable-selector-wrap{ data: { issuable_type: issuable.to_ability_name } } + = template_dropdown_tag(issuable) do + %ul.dropdown-footer-list + %li + %a.no-template + No template + %a.reset-template + Reset template diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index 3d2122a159c..264391fe84f 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -32,7 +32,7 @@ = link_to project_path(project), class: dom_class(project) do - if avatar .dash-project-avatar - .image-container.s40 + .avatar-container.s40 - if use_creator_avatar = image_tag avatar_icon(project.creator.email, 40), class: "avatar s40", alt:'' - else diff --git a/app/views/users/_groups.html.haml b/app/views/users/_groups.html.haml index 78f253f9023..eff6c80d144 100644 --- a/app/views/users/_groups.html.haml +++ b/app/views/users/_groups.html.haml @@ -1,5 +1,5 @@ .clearfix - groups.each do |group| = link_to group, class: 'profile-groups-avatars inline', title: group.name do - .image-container.s40 + .avatar-container.s40 = image_tag group_icon(group), class: 'avatar group-avatar s40' diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb index 65f8093b5b0..d369b639ae9 100644 --- a/app/workers/git_garbage_collect_worker.rb +++ b/app/workers/git_garbage_collect_worker.rb @@ -1,17 +1,58 @@ class GitGarbageCollectWorker include Sidekiq::Worker - include Gitlab::ShellAdapter include DedicatedSidekiqQueue + include Gitlab::CurrentSettings sidekiq_options retry: false - def perform(project_id) + def perform(project_id, task = :gc, lease_key = nil, lease_uuid = nil) project = Project.find(project_id) + task = task.to_sym + + cmd = command(task) + repo_path = project.repository.path_to_repo + description = "'#{cmd.join(' ')}' in #{repo_path}" + + Gitlab::GitLogger.info(description) + + output, status = Gitlab::Popen.popen(cmd, repo_path) + Gitlab::GitLogger.error("#{description} failed:\n#{output}") unless status.zero? - gitlab_shell.gc(project.repository_storage_path, project.path_with_namespace) # Refresh the branch cache in case garbage collection caused a ref lookup to fail + flush_ref_caches(project) if task == :gc + ensure + Gitlab::ExclusiveLease.cancel(lease_key, lease_uuid) if lease_key.present? && lease_uuid.present? + end + + private + + def command(task) + case task + when :gc + git(write_bitmaps: bitmaps_enabled?) + %w[gc] + when :full_repack + git(write_bitmaps: bitmaps_enabled?) + %w[repack -A -d --pack-kept-objects] + when :incremental_repack + # Normal git repack fails when bitmaps are enabled. It is impossible to + # create a bitmap here anyway. + git(write_bitmaps: false) + %w[repack -d] + else + raise "Invalid gc task: #{task.inspect}" + end + end + + def flush_ref_caches(project) project.repository.after_create_branch project.repository.branch_names project.repository.has_visible_content? end + + def bitmaps_enabled? + current_application_settings.housekeeping_bitmaps_enabled + end + + def git(write_bitmaps:) + config_value = write_bitmaps ? 'true' : 'false' + %W[git -c repack.writeBitmaps=#{config_value}] + end end diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb new file mode 100644 index 00000000000..071741fbacd --- /dev/null +++ b/app/workers/process_commit_worker.rb @@ -0,0 +1,67 @@ +# Worker for processing individiual commit messages pushed to a repository. +# +# Jobs for this worker are scheduled for every commit that is being pushed. As a +# result of this the workload of this worker should be kept to a bare minimum. +# Consider using an extra worker if you need to add any extra (and potentially +# slow) processing of commits. +class ProcessCommitWorker + include Sidekiq::Worker + include DedicatedSidekiqQueue + + # project_id - The ID of the project this commit belongs to. + # user_id - The ID of the user that pushed the commit. + # commit_sha - The SHA1 of the commit to process. + # default - The data was pushed to the default branch. + def perform(project_id, user_id, commit_sha, default = false) + project = Project.find_by(id: project_id) + + return unless project + + user = User.find_by(id: user_id) + + return unless user + + commit = find_commit(project, commit_sha) + + return unless commit + + author = commit.author || user + + process_commit_message(project, commit, user, author, default) + + update_issue_metrics(commit, author) + end + + def process_commit_message(project, commit, user, author, default = false) + closed_issues = default ? commit.closes_issues(user) : [] + + unless closed_issues.empty? + close_issues(project, user, author, commit, closed_issues) + end + + commit.create_cross_references!(author, closed_issues) + end + + def close_issues(project, user, author, commit, issues) + # We don't want to run permission related queries for every single issue, + # therefor we use IssueCollection here and skip the authorization check in + # Issues::CloseService#execute. + IssueCollection.new(issues).updatable_by_user(user).each do |issue| + Issues::CloseService.new(project, author). + close_issue(issue, commit: commit) + end + end + + def update_issue_metrics(commit, author) + mentioned_issues = commit.all_references(author).issues + + Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil). + update_all(first_mentioned_in_commit_at: commit.committed_date) + end + + private + + def find_commit(project, sha) + project.commit(sha) + end +end diff --git a/changelogs/unreleased/add-api-label-id.yml b/changelogs/unreleased/add-api-label-id.yml new file mode 100644 index 00000000000..3af4f5e677d --- /dev/null +++ b/changelogs/unreleased/add-api-label-id.yml @@ -0,0 +1,4 @@ +--- +title: Expose label IDs in API +merge_request: 7275 +author: Rares Sfirlogea diff --git a/changelogs/unreleased/add-project-import-data-index.yml b/changelogs/unreleased/add-project-import-data-index.yml new file mode 100644 index 00000000000..f5e4005f544 --- /dev/null +++ b/changelogs/unreleased/add-project-import-data-index.yml @@ -0,0 +1,4 @@ +--- +title: Add an index for project_id in project_import_data to improve performance +merge_request: +author: diff --git a/changelogs/unreleased/api-label-priorities.yml b/changelogs/unreleased/api-label-priorities.yml new file mode 100644 index 00000000000..85b6c2761bb --- /dev/null +++ b/changelogs/unreleased/api-label-priorities.yml @@ -0,0 +1,4 @@ +--- +title: API: Ability to retrieve version information +merge_request: 7286 +author: Robert Schilling diff --git a/changelogs/unreleased/broken-link-frontend-dev-guide.yml b/changelogs/unreleased/broken-link-frontend-dev-guide.yml new file mode 100644 index 00000000000..d7b6f4a7701 --- /dev/null +++ b/changelogs/unreleased/broken-link-frontend-dev-guide.yml @@ -0,0 +1,4 @@ +--- +title: Fix broken link to observatory cli on Frontend Dev Guide +merge_request: +author: Sam Rose diff --git a/changelogs/unreleased/git-gc-improvements.yml b/changelogs/unreleased/git-gc-improvements.yml new file mode 100644 index 00000000000..f15e667ce87 --- /dev/null +++ b/changelogs/unreleased/git-gc-improvements.yml @@ -0,0 +1,4 @@ +--- +title: Finer-grained Git gargage collection +merge_request: 6588 +author: diff --git a/changelogs/unreleased/issue_23032.yml b/changelogs/unreleased/issue_23032.yml new file mode 100644 index 00000000000..d376cf52112 --- /dev/null +++ b/changelogs/unreleased/issue_23032.yml @@ -0,0 +1,4 @@ +--- +title: Allow to test JIRA service settings without having a repository +merge_request: +author: diff --git a/changelogs/unreleased/process-commits-using-sidekiq.yml b/changelogs/unreleased/process-commits-using-sidekiq.yml new file mode 100644 index 00000000000..9f596e6a584 --- /dev/null +++ b/changelogs/unreleased/process-commits-using-sidekiq.yml @@ -0,0 +1,4 @@ +--- +title: Process commits using a dedicated Sidekiq worker +merge_request: 6802 +author: diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index f36fe893fd0..0aec8aedf72 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -21,6 +21,7 @@ - [post_receive, 5] - [merge, 5] - [update_merge_requests, 3] + - [process_commit, 2] - [new_note, 2] - [build, 2] - [pipeline, 2] diff --git a/db/migrate/20161031155516_add_housekeeping_to_application_settings.rb b/db/migrate/20161031155516_add_housekeeping_to_application_settings.rb new file mode 100644 index 00000000000..5a451fb575b --- /dev/null +++ b/db/migrate/20161031155516_add_housekeeping_to_application_settings.rb @@ -0,0 +1,32 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddHousekeepingToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + disable_ddl_transaction! + + def up + add_column_with_default(:application_settings, :housekeeping_enabled, :boolean, default: true, allow_null: false) + add_column_with_default(:application_settings, :housekeeping_bitmaps_enabled, :boolean, default: true, allow_null: false) + add_column_with_default(:application_settings, :housekeeping_incremental_repack_period, :integer, default: 10, allow_null: false) + add_column_with_default(:application_settings, :housekeeping_full_repack_period, :integer, default: 50, allow_null: false) + add_column_with_default(:application_settings, :housekeeping_gc_period, :integer, default: 200, allow_null: false) + end + + def down + remove_column(:application_settings, :housekeeping_enabled, :boolean, default: true, allow_null: false) + remove_column(:application_settings, :housekeeping_bitmaps_enabled, :boolean, default: true, allow_null: false) + remove_column(:application_settings, :housekeeping_incremental_repack_period, :integer, default: 10, allow_null: false) + remove_column(:application_settings, :housekeeping_full_repack_period, :integer, default: 50, allow_null: false) + remove_column(:application_settings, :housekeeping_gc_period, :integer, default: 200, allow_null: false) + end +end diff --git a/db/migrate/20161103171205_rename_repository_storage_column.rb b/db/migrate/20161103171205_rename_repository_storage_column.rb index e9f992793b4..93280573939 100644 --- a/db/migrate/20161103171205_rename_repository_storage_column.rb +++ b/db/migrate/20161103171205_rename_repository_storage_column.rb @@ -5,12 +5,12 @@ class RenameRepositoryStorageColumn < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers # Set this constant to true if this migration requires downtime. - DOWNTIME = false + DOWNTIME = true # When a migration requires downtime you **must** uncomment the following # constant and define a short and easy to understand explanation as to why the # migration requires downtime. - # DOWNTIME_REASON = '' + DOWNTIME_REASON = 'Renaming the application_settings.repository_storage column' # When using the methods "add_concurrent_index" or "add_column_with_default" # you must disable the use of transactions as these methods can not run in an diff --git a/db/migrate/20161106185620_add_project_import_data_project_index.rb b/db/migrate/20161106185620_add_project_import_data_project_index.rb new file mode 100644 index 00000000000..750a6a8c51e --- /dev/null +++ b/db/migrate/20161106185620_add_project_import_data_project_index.rb @@ -0,0 +1,12 @@ +class AddProjectImportDataProjectIndex < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def change + add_concurrent_index :project_import_data, :project_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 5476b0c93e5..60e7b26e29e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161103171205) do +ActiveRecord::Schema.define(version: 20161106185620) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -98,6 +98,11 @@ ActiveRecord::Schema.define(version: 20161103171205) do t.text "help_page_text_html" t.text "shared_runners_text_html" t.text "after_sign_up_text_html" + t.boolean "housekeeping_enabled", default: true, null: false + t.boolean "housekeeping_bitmaps_enabled", default: true, null: false + t.integer "housekeeping_incremental_repack_period", default: 10, null: false + t.integer "housekeeping_full_repack_period", default: 50, null: false + t.integer "housekeeping_gc_period", default: 200, null: false end create_table "audit_events", force: :cascade do |t| @@ -867,6 +872,8 @@ ActiveRecord::Schema.define(version: 20161103171205) do t.string "encrypted_credentials_salt" end + add_index "project_import_data", ["project_id"], name: "index_project_import_data_on_project_id", using: :btree + create_table "projects", force: :cascade do |t| t.string "name" t.string "path" diff --git a/doc/administration/housekeeping.md b/doc/administration/housekeeping.md index ad1fa98b63b..f846c06ca42 100644 --- a/doc/administration/housekeeping.md +++ b/doc/administration/housekeeping.md @@ -3,6 +3,14 @@ > [Introduced][ce-2371] in GitLab 8.4. --- +## Automatic housekeeping + +GitLab automatically runs `git gc` and `git repack` on repositories +after Git pushes. If needed you can change how often this happens, or +to turn it off, go to **Admin area > Settings** +(`/admin/application_settings`). + +## Manual housekeeping The housekeeping function runs `git gc` ([man page][man]) on the current project Git repository. diff --git a/doc/api/labels.md b/doc/api/labels.md index 656232cc940..78686fdcad4 100644 --- a/doc/api/labels.md +++ b/doc/api/labels.md @@ -20,46 +20,61 @@ Example response: ```json [ - { - "name" : "bug", - "color" : "#d9534f", - "description": "Bug reported by user", - "open_issues_count": 1, - "closed_issues_count": 0, - "open_merge_requests_count": 1 - }, - { - "color" : "#d9534f", - "name" : "confirmed", - "description": "Confirmed issue", - "open_issues_count": 2, - "closed_issues_count": 5, - "open_merge_requests_count": 0 - }, - { - "name" : "critical", - "color" : "#d9534f", - "description": "Critical issue. Need fix ASAP", - "open_issues_count": 1, - "closed_issues_count": 3, - "open_merge_requests_count": 1 - }, - { - "name" : "documentation", - "color" : "#f0ad4e", - "description": "Issue about documentation", - "open_issues_count": 1, - "closed_issues_count": 0, - "open_merge_requests_count": 2 - }, - { - "color" : "#5cb85c", - "name" : "enhancement", - "description": "Enhancement proposal", - "open_issues_count": 1, - "closed_issues_count": 0, - "open_merge_requests_count": 1 - } + { + "id" : 1, + "name" : "bug", + "color" : "#d9534f", + "description": "Bug reported by user", + "open_issues_count": 1, + "closed_issues_count": 0, + "open_merge_requests_count": 1, + "subscribed": false, + "priority": 10 + }, + { + "id" : 4, + "color" : "#d9534f", + "name" : "confirmed", + "description": "Confirmed issue", + "open_issues_count": 2, + "closed_issues_count": 5, + "open_merge_requests_count": 0, + "subscribed": false, + "priority": null + }, + { + "id" : 7, + "name" : "critical", + "color" : "#d9534f", + "description": "Critical issue. Need fix ASAP", + "open_issues_count": 1, + "closed_issues_count": 3, + "open_merge_requests_count": 1, + "subscribed": false, + "priority": null + }, + { + "id" : 8, + "name" : "documentation", + "color" : "#f0ad4e", + "description": "Issue about documentation", + "open_issues_count": 1, + "closed_issues_count": 0, + "open_merge_requests_count": 2, + "subscribed": false, + "priority": null + }, + { + "id" : 9, + "color" : "#5cb85c", + "name" : "enhancement", + "description": "Enhancement proposal", + "open_issues_count": 1, + "closed_issues_count": 0, + "open_merge_requests_count": 1, + "subscribed": true, + "priority": null + } ] ``` @@ -80,6 +95,7 @@ POST /projects/:id/labels | `name` | string | yes | The name of the label | | `color` | string | yes | The color of the label in 6-digit hex notation with leading `#` sign | | `description` | string | no | The description of the label | +| `priority` | integer | no | The priority of the label. Must be greater or equal than zero or `null` to remove the priority. | ```bash curl --data "name=feature&color=#5843AD" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/labels" @@ -89,9 +105,15 @@ Example response: ```json { - "name" : "feature", - "color" : "#5843AD", - "description":null + "id" : 10, + "name" : "feature", + "color" : "#5843AD", + "description":null, + "open_issues_count": 0, + "closed_issues_count": 0, + "open_merge_requests_count": 0, + "subscribed": false, + "priority": null } ``` @@ -120,14 +142,15 @@ Example response: ```json { - "title" : "feature", - "color" : "#5843AD", - "description": "New feature proposal", - "updated_at" : "2015-11-03T21:22:30.737Z", - "template" : false, - "project_id" : 1, - "created_at" : "2015-11-03T21:22:30.737Z", - "id" : 9 + "id" : 1, + "name" : "bug", + "color" : "#d9534f", + "description": "Bug reported by user", + "open_issues_count": 1, + "closed_issues_count": 0, + "open_merge_requests_count": 1, + "subscribed": false, + "priority": null } ``` @@ -151,6 +174,8 @@ PUT /projects/:id/labels | `new_name` | string | yes if `color` is not provided | The new name of the label | | `color` | string | yes if `new_name` is not provided | The new color of the label in 6-digit hex notation with leading `#` sign | | `description` | string | no | The new description of the label | +| `priority` | integer | no | The new priority of the label. Must be greater or equal than zero or `null` to remove the priority. | + ```bash curl --request PUT --data "name=documentation&new_name=docs&color=#8E44AD&description=Documentation" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/labels" @@ -160,9 +185,15 @@ Example response: ```json { - "color" : "#8E44AD", - "name" : "docs", - "description": "Documentation" + "id" : 8, + "name" : "docs", + "color" : "#8E44AD", + "description": "Documentation", + "open_issues_count": 1, + "closed_issues_count": 0, + "open_merge_requests_count": 2, + "subscribed": false, + "priority": null } ``` @@ -191,13 +222,15 @@ Example response: ```json { - "name": "Docs", - "color": "#cc0033", - "description": "", - "open_issues_count": 0, - "closed_issues_count": 0, - "open_merge_requests_count": 0, - "subscribed": true + "id" : 1, + "name" : "bug", + "color" : "#d9534f", + "description": "Bug reported by user", + "open_issues_count": 1, + "closed_issues_count": 0, + "open_merge_requests_count": 1, + "subscribed": true, + "priority": null } ``` @@ -226,12 +259,14 @@ Example response: ```json { - "name": "Docs", - "color": "#cc0033", - "description": "", - "open_issues_count": 0, - "closed_issues_count": 0, - "open_merge_requests_count": 0, - "subscribed": false + "id" : 1, + "name" : "bug", + "color" : "#d9534f", + "description": "Bug reported by user", + "open_issues_count": 1, + "closed_issues_count": 0, + "open_merge_requests_count": 1, + "subscribed": false, + "priority": null } ``` diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 1d7d9127a64..ec8f2d6531c 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -228,7 +228,7 @@ For our currently-supported browsers, see our [requirements][requirements]. [page-specific-js-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/13bb9ed77f405c5f6ee4fdbc964ecf635c9a223f/app/views/projects/graphs/_head.html.haml#L6-8 [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools [audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules -[observatory-cli]: https://github.com/mozilla/http-observatory-cli) +[observatory-cli]: https://github.com/mozilla/http-observatory-cli [qualys-ssl]: https://www.ssllabs.com/ssltest/analyze.html [secure_headers]: https://github.com/twitter/secureheaders [mdn-csp]: https://developer.mozilla.org/en-US/docs/Web/Security/CSP diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index 2574c2c0472..bbcd26477f3 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -66,6 +66,12 @@ producing errors whenever it tries to use the `dummy` column. As a result of the above downtime _is_ required when removing a column, even when using PostgreSQL. +## Renaming Columns + +Renaming columns requires downtime as running GitLab instances will continue +using the old column name until a new version is deployed. This can result +in the instance producing errors, which in turn can impact the user experience. + ## Changing Column Constraints Generally changing column constraints requires checking all rows in the table to diff --git a/doc/install/installation.md b/doc/install/installation.md index 7e947e4b2ba..b5e2640b380 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -403,7 +403,7 @@ If you are not using Linux you may have to run `gmake` instead of cd /home/git sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-workhorse.git cd gitlab-workhorse - sudo -u git -H git checkout v0.8.5 + sudo -u git -H git checkout v1.0.0 sudo -u git -H make ### Initialize Database and Activate Advanced Features diff --git a/doc/university/README.md b/doc/university/README.md index 510b753f70d..49714e4fb59 100644 --- a/doc/university/README.md +++ b/doc/university/README.md @@ -200,7 +200,7 @@ The curriculum is composed of GitLab videos, screencasts, presentations, project ## 4. <a name="external"></a> External Articles -1. [2011 WSJ article by Mark Andreeson - Software is Eating the World](http://www.wsj.com/articles/SB10001424053111903480904576512250915629460) +1. [2011 WSJ article by Marc Andreessen - Software is Eating the World](http://www.wsj.com/articles/SB10001424053111903480904576512250915629460) 1. [2014 Blog post by Chris Dixon - Software eats software development](http://cdixon.org/2014/04/13/software-eats-software-development/) 1. [2015 Venture Beat article - Actually, Open Source is Eating the World](http://venturebeat.com/2015/12/06/its-actually-open-source-software-thats-eating-the-world/) diff --git a/doc/update/8.13-to-8.14.md b/doc/update/8.13-to-8.14.md index 787511fd6cf..46ea19d11d0 100644 --- a/doc/update/8.13-to-8.14.md +++ b/doc/update/8.13-to-8.14.md @@ -84,7 +84,7 @@ GitLab 8.1. ```bash cd /home/git/gitlab-workhorse sudo -u git -H git fetch --all -sudo -u git -H git checkout v0.8.5 +sudo -u git -H git checkout v1.0.0 sudo -u git -H make ``` diff --git a/features/snippets/public_snippets.feature b/features/snippets/public_snippets.feature deleted file mode 100644 index c2afb63b6d8..00000000000 --- a/features/snippets/public_snippets.feature +++ /dev/null @@ -1,10 +0,0 @@ -Feature: Public snippets - Scenario: Unauthenticated user should see public snippets - Given There is public "Personal snippet one" snippet - And I visit snippet page "Personal snippet one" - Then I should see snippet "Personal snippet one" - - Scenario: Unauthenticated user should see raw public snippets - Given There is public "Personal snippet one" snippet - And I visit snippet raw page "Personal snippet one" - Then I should see raw snippet "Personal snippet one" diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index 4df4e89f5b9..35b71599708 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -210,7 +210,7 @@ module SharedDiffNote end step 'I click side-by-side diff button' do - find('#parallel-diff-btn').click + find('#parallel-diff-btn').trigger('click') end step 'I see side-by-side diff button' do diff --git a/features/steps/snippets/public_snippets.rb b/features/steps/snippets/public_snippets.rb deleted file mode 100644 index 2ebdca5ed30..00000000000 --- a/features/steps/snippets/public_snippets.rb +++ /dev/null @@ -1,25 +0,0 @@ -class Spinach::Features::PublicSnippets < Spinach::FeatureSteps - include SharedAuthentication - include SharedPaths - include SharedSnippet - - step 'I should see snippet "Personal snippet one"' do - expect(page).to have_no_xpath("//i[@class='public-snippet']") - end - - step 'I should see raw snippet "Personal snippet one"' do - expect(page).to have_text(snippet.content) - end - - step 'I visit snippet page "Personal snippet one"' do - visit snippet_path(snippet) - end - - step 'I visit snippet raw page "Personal snippet one"' do - visit raw_snippet_path(snippet) - end - - def snippet - @snippet ||= PersonalSnippet.find_by!(title: "Personal snippet one") - end -end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 01e31f6f7d1..1942aeea656 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -433,11 +433,14 @@ module API end class LabelBasic < Grape::Entity - expose :name, :color, :description + expose :id, :name, :color, :description end class Label < LabelBasic expose :open_issues_count, :closed_issues_count, :open_merge_requests_count + expose :priority do |label, options| + label.priority(options[:project]) + end expose :subscribed do |label, options| label.subscribed?(options[:current_user]) diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 238cea00fba..97218054f37 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -11,7 +11,7 @@ module API success Entities::Label end get ':id/labels' do - present available_labels, with: Entities::Label, current_user: current_user + present available_labels, with: Entities::Label, current_user: current_user, project: user_project end desc 'Create a new label' do @@ -21,6 +21,7 @@ module API requires :name, type: String, desc: 'The name of the label to be created' requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" optional :description, type: String, desc: 'The description of label to be created' + optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true end post ':id/labels' do authorize! :admin_label, user_project @@ -28,10 +29,15 @@ module API label = available_labels.find_by(title: params[:name]) conflict!('Label already exists') if label - label = user_project.labels.create(declared(params, include_parent_namespaces: false).to_h) + priority = params.delete(:priority) + label_params = declared(params, + include_parent_namespaces: false, + include_missing: false).to_h + label = user_project.labels.create(label_params) if label.valid? - present label, with: Entities::Label, current_user: current_user + label.prioritize!(user_project, priority) if priority + present label, with: Entities::Label, current_user: current_user, project: user_project else render_validation_error!(label) end @@ -49,7 +55,7 @@ module API label = user_project.labels.find_by(title: params[:name]) not_found!('Label') unless label - present label.destroy, with: Entities::Label, current_user: current_user + present label.destroy, with: Entities::Label, current_user: current_user, project: user_project end desc 'Update an existing label. At least one optional parameter is required.' do @@ -60,7 +66,8 @@ module API optional :new_name, type: String, desc: 'The new name of the label' optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" optional :description, type: String, desc: 'The new description of label' - at_least_one_of :new_name, :color, :description + optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true + at_least_one_of :new_name, :color, :description, :priority end put ':id/labels' do authorize! :admin_label, user_project @@ -68,17 +75,25 @@ module API label = user_project.labels.find_by(title: params[:name]) not_found!('Label not found') unless label - update_params = declared(params, - include_parent_namespaces: false, - include_missing: false).to_h + update_priority = params.key?(:priority) + priority = params.delete(:priority) + label_params = declared(params, + include_parent_namespaces: false, + include_missing: false).to_h # Rename new name to the actual label attribute name - update_params['name'] = update_params.delete('new_name') if update_params.key?('new_name') + label_params[:name] = label_params.delete('new_name') if label_params.key?('new_name') - if label.update(update_params) - present label, with: Entities::Label, current_user: current_user - else - render_validation_error!(label) + render_validation_error!(label) unless label.update(label_params) + + if update_priority + if priority.nil? + label.unprioritize!(user_project) + else + label.prioritize!(user_project, priority) + end end + + present label, with: Entities::Label, current_user: current_user, project: user_project end end end diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb index ce048a36fa0..f31fb6c3f71 100644 --- a/lib/banzai/renderer.rb +++ b/lib/banzai/renderer.rb @@ -46,7 +46,7 @@ module Banzai return html if html.present? html = cacheless_render_field(object, field) - object.update_column(html_field, html) unless object.new_record? || object.destroyed? + update_object(object, html_field, html) unless object.new_record? || object.destroyed? html end @@ -166,5 +166,9 @@ module Banzai return unless cache_key Rails.cache.send(:expanded_key, full_cache_key(cache_key, pipeline_name)) end + + def update_object(object, html_field, html) + object.update_column(html_field, html) + end end end diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb index 9cec71a3222..82e194c1af1 100644 --- a/lib/gitlab/backend/shell.rb +++ b/lib/gitlab/backend/shell.rb @@ -127,19 +127,6 @@ module Gitlab 'rm-project', storage, "#{name}.git"]) end - # Gc repository - # - # storage - project storage path - # path - project path with namespace - # - # Ex. - # gc("/path/to/storage", "gitlab/gitlab-ci") - # - def gc(storage, path) - Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'gc', - storage, "#{path}.git"]) - end - # Add new key to gitlab-shell # # Ex. diff --git a/lib/gitlab/exclusive_lease.rb b/lib/gitlab/exclusive_lease.rb index 7e8f35e9298..2dd42704396 100644 --- a/lib/gitlab/exclusive_lease.rb +++ b/lib/gitlab/exclusive_lease.rb @@ -1,66 +1,52 @@ +require 'securerandom' + module Gitlab # This class implements an 'exclusive lease'. We call it a 'lease' # because it has a set expiry time. We call it 'exclusive' because only # one caller may obtain a lease for a given key at a time. The # implementation is intended to work across GitLab processes and across - # servers. It is a 'cheap' alternative to using SQL queries and updates: + # servers. It is a cheap alternative to using SQL queries and updates: # you do not need to change the SQL schema to start using # ExclusiveLease. # - # It is important to choose the timeout wisely. If the timeout is very - # high (1 hour) then the throughput of your operation gets very low (at - # most once an hour). If the timeout is lower than how long your - # operation may take then you cannot count on exclusivity. For example, - # if the timeout is 10 seconds and you do an operation which may take 20 - # seconds then two overlapping operations may hold a lease for the same - # key at the same time. - # - # This class has no 'cancel' method. I originally decided against adding - # it because it would add complexity and a false sense of security. The - # complexity: instead of setting '1' we would have to set a UUID, and to - # delete it we would have to execute Lua on the Redis server to only - # delete the key if the value was our own UUID. Otherwise there is a - # chance that when you intend to cancel your lease you actually delete - # someone else's. The false sense of security: you cannot design your - # system to rely too much on the lease being cancelled after use because - # the calling (Ruby) process may crash or be killed. You _cannot_ count - # on begin/ensure blocks to cancel a lease, because the 'ensure' does - # not always run. Think of 'kill -9' from the Unicorn master for - # instance. - # - # If you find that leases are getting in your way, ask yourself: would - # it be enough to lower the lease timeout? Another thing that might be - # appropriate is to only use a lease for bulk/automated operations, and - # to ignore the lease when you get a single 'manual' user request (a - # button click). - # class ExclusiveLease + LUA_CANCEL_SCRIPT = <<-EOS + local key, uuid = KEYS[1], ARGV[1] + if redis.call("get", key) == uuid then + redis.call("del", key) + end + EOS + + def self.cancel(key, uuid) + Gitlab::Redis.with do |redis| + redis.eval(LUA_CANCEL_SCRIPT, keys: [redis_key(key)], argv: [uuid]) + end + end + + def self.redis_key(key) + "gitlab:exclusive_lease:#{key}" + end + def initialize(key, timeout:) - @key, @timeout = key, timeout + @redis_key = self.class.redis_key(key) + @timeout = timeout + @uuid = SecureRandom.uuid end - # Try to obtain the lease. Return true on success, + # Try to obtain the lease. Return lease UUID on success, # false if the lease is already taken. def try_obtain # Performing a single SET is atomic Gitlab::Redis.with do |redis| - !!redis.set(redis_key, '1', nx: true, ex: @timeout) + redis.set(@redis_key, @uuid, nx: true, ex: @timeout) && @uuid end end # Returns true if the key for this lease is set. def exists? Gitlab::Redis.with do |redis| - redis.exists(redis_key) + redis.exists(@redis_key) end end - - # No #cancel method. See comments above! - - private - - def redis_key - "gitlab:exclusive_lease:#{@key}" - end end end diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index ecc28799737..90cf38a8513 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -52,13 +52,14 @@ module Gitlab fetch_resources(:labels, repo, per_page: 100) do |labels| labels.each do |raw| begin - label = LabelFormatter.new(project, raw).create! - @labels[label.title] = label.id + LabelFormatter.new(project, raw).create! rescue => e errors << { type: :label, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message } end end end + + cache_labels! end def import_milestones @@ -234,6 +235,12 @@ module Gitlab end end + def cache_labels! + project.labels.select(:id, :title).find_each do |label| + @labels[label.title] = label.id + end + end + def fetch_resources(resource_type, *opts) return if imported?(resource_type) diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb index a92075fec8f..6cb8753e8fc 100644 --- a/spec/features/boards/boards_spec.rb +++ b/spec/features/boards/boards_spec.rb @@ -380,6 +380,25 @@ describe 'Issue Boards', feature: true, js: true do wait_for_board_cards(1, 5) end + + it 'creates new list from a new label' do + click_button 'Create new list' + + wait_for_ajax + + click_link 'Create new label' + + fill_in('new_label_name', with: 'Testing New Label') + + first('.suggest-colors a').click + + click_button 'Create' + + wait_for_ajax + wait_for_vue_resource + + expect(page).to have_selector('.board', count: 5) + end end end diff --git a/spec/features/projects/ref_switcher_spec.rb b/spec/features/projects/ref_switcher_spec.rb index b3ba40b35af..472491188c9 100644 --- a/spec/features/projects/ref_switcher_spec.rb +++ b/spec/features/projects/ref_switcher_spec.rb @@ -22,8 +22,20 @@ feature 'Ref switcher', feature: true, js: true do input.native.send_keys :down input.native.send_keys :down input.native.send_keys :enter + end + + expect(page).to have_title 'expand-collapse-files' + end + + it "user selects ref with special characters" do + click_button 'master' + wait_for_ajax - expect(page).to have_content 'expand-collapse-files' + page.within '.project-refs-form' do + page.fill_in 'Search branches and tags', with: "'test'" + click_link "'test'" end + + expect(page).to have_title "'test'" end end diff --git a/spec/features/security/project/snippet/internal_access_spec.rb b/spec/features/security/project/snippet/internal_access_spec.rb index db53a9cec97..49deacc5c74 100644 --- a/spec/features/security/project/snippet/internal_access_spec.rb +++ b/spec/features/security/project/snippet/internal_access_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe "Internal Project Snippets Access", feature: true do include AccessMatchers - let(:project) { create(:project, :internal) } + let(:project) { create(:empty_project, :internal) } let(:owner) { project.owner } let(:master) { create(:user) } @@ -48,31 +48,63 @@ describe "Internal Project Snippets Access", feature: true do it { is_expected.to be_denied_for :visitor } end - describe "GET /:project_path/snippets/:id for an internal snippet" do - subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) } + describe "GET /:project_path/snippets/:id" do + context "for an internal snippet" do + subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for owner } - it { is_expected.to be_allowed_for master } - it { is_expected.to be_allowed_for developer } - it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for guest } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :external } - it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } + end + + context "for a private snippet" do + subject { namespace_project_snippet_path(project.namespace, project, private_snippet) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } + end end - describe "GET /:project_path/snippets/:id for a private snippet" do - subject { namespace_project_snippet_path(project.namespace, project, private_snippet) } + describe "GET /:project_path/snippets/:id/raw" do + context "for an internal snippet" do + subject { raw_namespace_project_snippet_path(project.namespace, project, internal_snippet) } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for owner } - it { is_expected.to be_allowed_for master } - it { is_expected.to be_allowed_for developer } - it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for guest } - it { is_expected.to be_denied_for :user } - it { is_expected.to be_denied_for :external } - it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } + end + + context "for a private snippet" do + subject { raw_namespace_project_snippet_path(project.namespace, project, private_snippet) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } + end end end diff --git a/spec/features/security/project/snippet/private_access_spec.rb b/spec/features/security/project/snippet/private_access_spec.rb index d23d645c8e5..a1bfc076d99 100644 --- a/spec/features/security/project/snippet/private_access_spec.rb +++ b/spec/features/security/project/snippet/private_access_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe "Private Project Snippets Access", feature: true do include AccessMatchers - let(:project) { create(:project, :private) } + let(:project) { create(:empty_project, :private) } let(:owner) { project.owner } let(:master) { create(:user) } @@ -60,4 +60,18 @@ describe "Private Project Snippets Access", feature: true do it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end + + describe "GET /:project_path/snippets/:id/raw for a private snippet" do + subject { raw_namespace_project_snippet_path(project.namespace, project, private_snippet) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } + end end diff --git a/spec/features/security/project/snippet/public_access_spec.rb b/spec/features/security/project/snippet/public_access_spec.rb index e3665b6116a..30bcd87ef04 100644 --- a/spec/features/security/project/snippet/public_access_spec.rb +++ b/spec/features/security/project/snippet/public_access_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe "Public Project Snippets Access", feature: true do include AccessMatchers - let(:project) { create(:project, :public) } + let(:project) { create(:empty_project, :public) } let(:owner) { project.owner } let(:master) { create(:user) } @@ -49,45 +49,91 @@ describe "Public Project Snippets Access", feature: true do it { is_expected.to be_denied_for :visitor } end - describe "GET /:project_path/snippets/:id for a public snippet" do - subject { namespace_project_snippet_path(project.namespace, project, public_snippet) } + describe "GET /:project_path/snippets/:id" do + context "for a public snippet" do + subject { namespace_project_snippet_path(project.namespace, project, public_snippet) } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for owner } - it { is_expected.to be_allowed_for master } - it { is_expected.to be_allowed_for developer } - it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for guest } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :external } - it { is_expected.to be_allowed_for :visitor } - end + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } + it { is_expected.to be_allowed_for :visitor } + end - describe "GET /:project_path/snippets/:id for an internal snippet" do - subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) } + context "for an internal snippet" do + subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for owner } - it { is_expected.to be_allowed_for master } - it { is_expected.to be_allowed_for developer } - it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for guest } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :external } - it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } + end + + context "for a private snippet" do + subject { namespace_project_snippet_path(project.namespace, project, private_snippet) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } + end end - describe "GET /:project_path/snippets/:id for a private snippet" do - subject { namespace_project_snippet_path(project.namespace, project, private_snippet) } + describe "GET /:project_path/snippets/:id/raw" do + context "for a public snippet" do + subject { raw_namespace_project_snippet_path(project.namespace, project, public_snippet) } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for owner } - it { is_expected.to be_allowed_for master } - it { is_expected.to be_allowed_for developer } - it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for guest } - it { is_expected.to be_denied_for :user } - it { is_expected.to be_denied_for :external } - it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } + it { is_expected.to be_allowed_for :visitor } + end + + context "for an internal snippet" do + subject { raw_namespace_project_snippet_path(project.namespace, project, internal_snippet) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } + end + + context "for a private snippet" do + subject { raw_namespace_project_snippet_path(project.namespace, project, private_snippet) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } + end end end diff --git a/spec/features/snippets/public_snippets_spec.rb b/spec/features/snippets/public_snippets_spec.rb new file mode 100644 index 00000000000..34300ccb940 --- /dev/null +++ b/spec/features/snippets/public_snippets_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +feature 'Public Snippets', feature: true do + scenario 'Unauthenticated user should see public snippets' do + public_snippet = create(:personal_snippet, :public) + + visit snippet_path(public_snippet) + + expect(page).to have_content(public_snippet.content) + end + + scenario 'Unauthenticated user should see raw public snippets' do + public_snippet = create(:personal_snippet, :public) + + visit raw_snippet_path(public_snippet) + + expect(page).to have_content(public_snippet.content) + end +end diff --git a/spec/javascripts/diff_comments_store_spec.js.es6 b/spec/javascripts/diff_comments_store_spec.js.es6 index 5d817802602..9b2845af608 100644 --- a/spec/javascripts/diff_comments_store_spec.js.es6 +++ b/spec/javascripts/diff_comments_store_spec.js.es6 @@ -92,7 +92,6 @@ it('is unresolved with 2 notes', () => { const discussion = CommentsStore.state['a']; createDiscussion(2, false); - console.log(discussion.isResolved()); expect(discussion.isResolved()).toBe(false); }); diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb index f826d0d1b04..4b08a02ec73 100644 --- a/spec/lib/gitlab/backend/shell_spec.rb +++ b/spec/lib/gitlab/backend/shell_spec.rb @@ -14,7 +14,6 @@ describe Gitlab::Shell, lib: true do it { is_expected.to respond_to :add_repository } it { is_expected.to respond_to :remove_repository } it { is_expected.to respond_to :fork_repository } - it { is_expected.to respond_to :gc } it { is_expected.to respond_to :add_namespace } it { is_expected.to respond_to :rm_namespace } it { is_expected.to respond_to :mv_namespace } diff --git a/spec/lib/gitlab/exclusive_lease_spec.rb b/spec/lib/gitlab/exclusive_lease_spec.rb index 6b3bd08b978..a366d68a146 100644 --- a/spec/lib/gitlab/exclusive_lease_spec.rb +++ b/spec/lib/gitlab/exclusive_lease_spec.rb @@ -5,32 +5,47 @@ describe Gitlab::ExclusiveLease, type: :redis do describe '#try_obtain' do it 'cannot obtain twice before the lease has expired' do - lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600) - expect(lease.try_obtain).to eq(true) + lease = described_class.new(unique_key, timeout: 3600) + expect(lease.try_obtain).to be_present expect(lease.try_obtain).to eq(false) end it 'can obtain after the lease has expired' do timeout = 1 - lease = Gitlab::ExclusiveLease.new(unique_key, timeout: timeout) + lease = described_class.new(unique_key, timeout: timeout) lease.try_obtain # start the lease sleep(2 * timeout) # lease should have expired now - expect(lease.try_obtain).to eq(true) + expect(lease.try_obtain).to be_present end end describe '#exists?' do it 'returns true for an existing lease' do - lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600) + lease = described_class.new(unique_key, timeout: 3600) lease.try_obtain expect(lease.exists?).to eq(true) end it 'returns false for a lease that does not exist' do - lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600) + lease = described_class.new(unique_key, timeout: 3600) expect(lease.exists?).to eq(false) end end + + describe '.cancel' do + it 'can cancel a lease' do + uuid = new_lease(unique_key) + expect(uuid).to be_present + expect(new_lease(unique_key)).to eq(false) + + described_class.cancel(unique_key, uuid) + expect(new_lease(unique_key)).to be_present + end + + def new_lease(key) + described_class.new(key, timeout: 3600).try_obtain + end + end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 2b76e056f3c..b950fcdd81a 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -98,6 +98,24 @@ describe ApplicationSetting, models: true do end end end + + context 'housekeeping settings' do + it { is_expected.not_to allow_value(0).for(:housekeeping_incremental_repack_period) } + + it 'wants the full repack period to be longer than the incremental repack period' do + subject.housekeeping_incremental_repack_period = 2 + subject.housekeeping_full_repack_period = 1 + + expect(subject).not_to be_valid + end + + it 'wants the gc period to be longer than the full repack period' do + subject.housekeeping_full_repack_period = 2 + subject.housekeeping_gc_period = 1 + + expect(subject).not_to be_valid + end + end end context 'restricted signup domains' do diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index a59d30687f6..a9603074c32 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -341,4 +341,25 @@ describe Issue, "Issuable" do expect(Issue.with_label([bug.title, enhancement.title])).to match_array([issue2]) end end + + describe '#assignee_or_author?' do + let(:user) { build(:user, id: 1) } + let(:issue) { build(:issue) } + + it 'returns true for a user that is assigned to an issue' do + issue.assignee = user + + expect(issue.assignee_or_author?(user)).to eq(true) + end + + it 'returns true for a user that is the author of an issue' do + issue.author = user + + expect(issue.assignee_or_author?(user)).to eq(true) + end + + it 'returns false for a user that is not the assignee or author' do + expect(issue.assignee_or_author?(user)).to eq(false) + end + end end diff --git a/spec/models/external_issue_spec.rb b/spec/models/external_issue_spec.rb index ebba6e14578..2debe1289a3 100644 --- a/spec/models/external_issue_spec.rb +++ b/spec/models/external_issue_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe ExternalIssue, models: true do - let(:project) { double('project', to_reference: 'namespace1/project1') } + let(:project) { double('project', id: 1, to_reference: 'namespace1/project1') } let(:issue) { described_class.new('EXT-1234', project) } describe 'modules' do @@ -36,4 +36,10 @@ describe ExternalIssue, models: true do end end end + + describe '#project_id' do + it 'returns the ID of the project' do + expect(issue.project_id).to eq(project.id) + end + end end diff --git a/spec/models/issue_collection_spec.rb b/spec/models/issue_collection_spec.rb new file mode 100644 index 00000000000..d742c814680 --- /dev/null +++ b/spec/models/issue_collection_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe IssueCollection do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:issue1) { create(:issue, project: project) } + let(:issue2) { create(:issue, project: project) } + let(:collection) { described_class.new([issue1, issue2]) } + + describe '#collection' do + it 'returns the issues in the same order as the input Array' do + expect(collection.collection).to eq([issue1, issue2]) + end + end + + describe '#updatable_by_user' do + context 'using an admin user' do + it 'returns all issues' do + user = create(:admin) + + expect(collection.updatable_by_user(user)).to eq([issue1, issue2]) + end + end + + context 'using a user that has no access to the project' do + it 'returns no issues when the user is not an assignee or author' do + expect(collection.updatable_by_user(user)).to be_empty + end + + it 'returns the issues the user is assigned to' do + issue1.assignee = user + + expect(collection.updatable_by_user(user)).to eq([issue1]) + end + + it 'returns the issues for which the user is the author' do + issue1.author = user + + expect(collection.updatable_by_user(user)).to eq([issue1]) + end + end + + context 'using a user that has reporter access to the project' do + it 'returns the issues of the project' do + project.team << [user, :reporter] + + expect(collection.updatable_by_user(user)).to eq([issue1, issue2]) + end + end + + context 'using a user that is the owner of a project' do + it 'returns the issues of the project' do + expect(collection.updatable_by_user(project.namespace.owner)). + to eq([issue1, issue2]) + end + end + end + + describe '#visible_to' do + it 'is an alias for updatable_by_user' do + updatable_by_user = described_class.instance_method(:updatable_by_user) + visible_to = described_class.instance_method(:visible_to) + + expect(visible_to).to eq(updatable_by_user) + end + end +end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index ee0e38bd373..05ee4a08391 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -33,6 +33,41 @@ describe JiraService, models: true do end end + describe '#can_test?' do + let(:jira_service) { described_class.new } + + it 'returns false if username is blank' do + allow(jira_service).to receive_messages( + url: 'http://jira.example.com', + username: '', + password: '12345678' + ) + + expect(jira_service.can_test?).to be_falsy + end + + it 'returns false if password is blank' do + allow(jira_service).to receive_messages( + url: 'http://jira.example.com', + username: 'tester', + password: '' + ) + + expect(jira_service.can_test?).to be_falsy + end + + it 'returns true if password and username are present' do + jira_service = described_class.new + allow(jira_service).to receive_messages( + url: 'http://jira.example.com', + username: 'tester', + password: '12345678' + ) + + expect(jira_service.can_test?).to be_truthy + end + end + describe "Execute" do let(:user) { create(:user) } let(:project) { create(:project) } @@ -46,16 +81,19 @@ describe JiraService, models: true do service_hook: true, url: 'http://jira.example.com', username: 'gitlab_jira_username', - password: 'gitlab_jira_password' + password: 'gitlab_jira_password', + project_key: 'GitLabProject' ) @jira_service.save - project_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123' - @transitions_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions' - @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment' + project_issues_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123' + @project_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/project/GitLabProject' + @transitions_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions' + @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment' - WebMock.stub_request(:get, project_url) + WebMock.stub_request(:get, @project_url) + WebMock.stub_request(:get, project_issues_url) WebMock.stub_request(:post, @transitions_url) WebMock.stub_request(:post, @comment_url) end @@ -99,6 +137,14 @@ describe JiraService, models: true do body: /this-is-a-custom-id/ ).once end + + context "when testing" do + it "tries to get jira project" do + @jira_service.execute(nil) + + expect(WebMock).to have_requested(:get, @project_url) + end + end end describe "Stored password invalidation" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ba47479a2e1..3b152e15b61 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1205,4 +1205,40 @@ describe User, models: true do expect(user.viewable_starred_projects).not_to include(private_project) end end + + describe '#projects_with_reporter_access_limited_to' do + let(:project1) { create(:project) } + let(:project2) { create(:project) } + let(:user) { create(:user) } + + before do + project1.team << [user, :reporter] + project2.team << [user, :guest] + end + + it 'returns the projects when using a single project ID' do + projects = user.projects_with_reporter_access_limited_to(project1.id) + + expect(projects).to eq([project1]) + end + + it 'returns the projects when using an Array of project IDs' do + projects = user.projects_with_reporter_access_limited_to([project1.id]) + + expect(projects).to eq([project1]) + end + + it 'returns the projects when using an ActiveRecord relation' do + projects = user. + projects_with_reporter_access_limited_to(Project.select(:id)) + + expect(projects).to eq([project1]) + end + + it 'does not return projects you do not have reporter access to' do + projects = user.projects_with_reporter_access_limited_to(project2.id) + + expect(projects).to be_empty + end + end end diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb new file mode 100644 index 00000000000..7591bfd1471 --- /dev/null +++ b/spec/policies/issue_policy_spec.rb @@ -0,0 +1,119 @@ +require 'spec_helper' + +describe IssuePolicy, models: true do + let(:user) { create(:user) } + + describe '#rules' do + context 'using a regular issue' do + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project) } + let(:policies) { described_class.abilities(user, issue).to_set } + + context 'with a regular user' do + it 'includes the read_issue permission' do + expect(policies).to include(:read_issue) + end + + it 'does not include the admin_issue permission' do + expect(policies).not_to include(:admin_issue) + end + + it 'does not include the update_issue permission' do + expect(policies).not_to include(:update_issue) + end + end + + context 'with a user that is a project reporter' do + before do + project.team << [user, :reporter] + end + + it 'includes the read_issue permission' do + expect(policies).to include(:read_issue) + end + + it 'includes the admin_issue permission' do + expect(policies).to include(:admin_issue) + end + + it 'includes the update_issue permission' do + expect(policies).to include(:update_issue) + end + end + + context 'with a user that is a project guest' do + before do + project.team << [user, :guest] + end + + it 'includes the read_issue permission' do + expect(policies).to include(:read_issue) + end + + it 'does not include the admin_issue permission' do + expect(policies).not_to include(:admin_issue) + end + + it 'does not include the update_issue permission' do + expect(policies).not_to include(:update_issue) + end + end + end + + context 'using a confidential issue' do + let(:issue) { create(:issue, :confidential) } + + context 'with a regular user' do + let(:policies) { described_class.abilities(user, issue).to_set } + + it 'does not include the read_issue permission' do + expect(policies).not_to include(:read_issue) + end + + it 'does not include the admin_issue permission' do + expect(policies).not_to include(:admin_issue) + end + + it 'does not include the update_issue permission' do + expect(policies).not_to include(:update_issue) + end + end + + context 'with a user that is a project member' do + let(:policies) { described_class.abilities(user, issue).to_set } + + before do + issue.project.team << [user, :reporter] + end + + it 'includes the read_issue permission' do + expect(policies).to include(:read_issue) + end + + it 'includes the admin_issue permission' do + expect(policies).to include(:admin_issue) + end + + it 'includes the update_issue permission' do + expect(policies).to include(:update_issue) + end + end + + context 'without a user' do + let(:policies) { described_class.abilities(nil, issue).to_set } + + it 'does not include the read_issue permission' do + expect(policies).not_to include(:read_issue) + end + + it 'does not include the admin_issue permission' do + expect(policies).not_to include(:admin_issue) + end + + it 'does not include the update_issue permission' do + expect(policies).not_to include(:update_issue) + end + end + end + end +end diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index f702dfaaf53..2ff90b6deac 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -6,6 +6,7 @@ describe API::API, api: true do let(:user) { create(:user) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let!(:label1) { create(:label, title: 'label1', project: project) } + let!(:priority_label) { create(:label, title: 'bug', project: project, priority: 3) } before do project.team << [user, :master] @@ -16,13 +17,27 @@ describe API::API, api: true do group = create(:group) group_label = create(:group_label, group: group) project.update(group: group) + expected_keys = [ + 'id', 'name', 'color', 'description', + 'open_issues_count', 'closed_issues_count', 'open_merge_requests_count', + 'subscribed', 'priority' + ] get api("/projects/#{project.id}/labels", user) expect(response).to have_http_status(200) expect(json_response).to be_an Array - expect(json_response.size).to eq(2) - expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, label1.name]) + expect(json_response.size).to eq(3) + expect(json_response.first.keys).to match_array expected_keys + expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, priority_label.name, label1.name]) + expect(json_response.last['name']).to eq(label1.name) + expect(json_response.last['color']).to be_present + expect(json_response.last['description']).to be_nil + expect(json_response.last['open_issues_count']).to eq(0) + expect(json_response.last['closed_issues_count']).to eq(0) + expect(json_response.last['open_merge_requests_count']).to eq(0) + expect(json_response.last['priority']).to be_nil + expect(json_response.last['subscribed']).to be_falsey end end @@ -31,21 +46,39 @@ describe API::API, api: true do post api("/projects/#{project.id}/labels", user), name: 'Foo', color: '#FFAABB', - description: 'test' + description: 'test', + priority: 2 + expect(response).to have_http_status(201) expect(json_response['name']).to eq('Foo') expect(json_response['color']).to eq('#FFAABB') expect(json_response['description']).to eq('test') + expect(json_response['priority']).to eq(2) end it 'returns created label when only required params' do post api("/projects/#{project.id}/labels", user), name: 'Foo & Bar', color: '#FFAABB' + expect(response.status).to eq(201) expect(json_response['name']).to eq('Foo & Bar') expect(json_response['color']).to eq('#FFAABB') expect(json_response['description']).to be_nil + expect(json_response['priority']).to be_nil + end + + it 'creates a prioritized label' do + post api("/projects/#{project.id}/labels", user), + name: 'Foo & Bar', + color: '#FFAABB', + priority: 3 + + expect(response.status).to eq(201) + expect(json_response['name']).to eq('Foo & Bar') + expect(json_response['color']).to eq('#FFAABB') + expect(json_response['description']).to be_nil + expect(json_response['priority']).to eq(3) end it 'returns a 400 bad request if name not given' do @@ -95,6 +128,15 @@ describe API::API, api: true do expect(json_response['message']).to eq('Label already exists') end + it 'returns 400 for invalid priority' do + post api("/projects/#{project.id}/labels", user), + name: 'Foo', + color: '#FFAAFFFF', + priority: 'foo' + + expect(response).to have_http_status(400) + end + it 'returns 409 if label already exists in project' do post api("/projects/#{project.id}/labels", user), name: 'label1', @@ -155,11 +197,43 @@ describe API::API, api: true do it 'returns 200 if description is changed' do put api("/projects/#{project.id}/labels", user), - name: 'label1', + name: 'bug', description: 'test' + expect(response).to have_http_status(200) - expect(json_response['name']).to eq(label1.name) + expect(json_response['name']).to eq(priority_label.name) expect(json_response['description']).to eq('test') + expect(json_response['priority']).to eq(3) + end + + it 'returns 200 if priority is changed' do + put api("/projects/#{project.id}/labels", user), + name: 'bug', + priority: 10 + + expect(response.status).to eq(200) + expect(json_response['name']).to eq(priority_label.name) + expect(json_response['priority']).to eq(10) + end + + it 'returns 200 if a priority is added' do + put api("/projects/#{project.id}/labels", user), + name: 'label1', + priority: 3 + + expect(response.status).to eq(200) + expect(json_response['name']).to eq(label1.name) + expect(json_response['priority']).to eq(3) + end + + it 'returns 200 if the priority is removed' do + put api("/projects/#{project.id}/labels", user), + name: priority_label.name, + priority: nil + + expect(response.status).to eq(200) + expect(json_response['name']).to eq(priority_label.name) + expect(json_response['priority']).to be_nil end it 'returns 404 if label does not exist' do @@ -178,7 +252,7 @@ describe API::API, api: true do it 'returns 400 if no new parameters given' do put api("/projects/#{project.id}/labels", user), name: 'label1' expect(response).to have_http_status(400) - expect(json_response['error']).to eq('new_name, color, description are missing, '\ + expect(json_response['error']).to eq('new_name, color, description, priority are missing, '\ 'at least one parameter must be provided') end @@ -206,6 +280,14 @@ describe API::API, api: true do expect(response).to have_http_status(400) expect(json_response['message']['color']).to eq(['must be a valid color code']) end + + it 'returns 400 for invalid priority' do + post api("/projects/#{project.id}/labels", user), + name: 'Foo', + priority: 'foo' + + expect(response).to have_http_status(400) + end end describe "POST /projects/:id/labels/:label_id/subscription" do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 45bc44ba172..cea7e6429f9 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -302,6 +302,9 @@ describe GitPushService, services: true do author_email: commit_author.email ) + allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit). + and_return(commit) + allow(project.repository).to receive(:commits_between).and_return([commit]) end @@ -357,6 +360,9 @@ describe GitPushService, services: true do committed_date: commit_time ) + allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit). + and_return(commit) + allow(project.repository).to receive(:commits_between).and_return([commit]) end @@ -393,6 +399,9 @@ describe GitPushService, services: true do allow(project.repository).to receive(:commits_between). and_return([closing_commit]) + allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit). + and_return(closing_commit) + project.team << [commit_author, :master] end @@ -538,9 +547,16 @@ describe GitPushService, services: true do let(:housekeeping) { Projects::HousekeepingService.new(project) } before do + # Flush any raw Redis data stored by the housekeeping code. + Gitlab::Redis.with { |conn| conn.flushall } + allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping) end + after do + Gitlab::Redis.with { |conn| conn.flushall } + end + it 'does not perform housekeeping when not needed' do expect(housekeeping).not_to receive(:execute) diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 5dfb33f4b28..4465f22a001 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -15,10 +15,39 @@ describe Issues::CloseService, services: true do end describe '#execute' do + let(:service) { described_class.new(project, user) } + + it 'checks if the user is authorized to update the issue' do + expect(service).to receive(:can?).with(user, :update_issue, issue). + and_call_original + + service.execute(issue) + end + + it 'does not close the issue when the user is not authorized to do so' do + allow(service).to receive(:can?).with(user, :update_issue, issue). + and_return(false) + + expect(service).not_to receive(:close_issue) + expect(service.execute(issue)).to eq(issue) + end + + it 'closes the issue when the user is authorized to do so' do + allow(service).to receive(:can?).with(user, :update_issue, issue). + and_return(true) + + expect(service).to receive(:close_issue). + with(issue, commit: nil, notifications: true, system_note: true) + + service.execute(issue) + end + end + + describe '#close_issue' do context "valid params" do before do perform_enqueued_jobs do - described_class.new(project, user).execute(issue) + described_class.new(project, user).close_issue(issue) end end @@ -41,24 +70,12 @@ describe Issues::CloseService, services: true do end end - context 'current user is not authorized to close issue' do - before do - perform_enqueued_jobs do - described_class.new(project, guest).execute(issue) - end - end - - it 'does not close the issue' do - expect(issue).to be_open - end - end - context 'when issue is not confidential' do it 'executes issue hooks' do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) - described_class.new(project, user).execute(issue) + described_class.new(project, user).close_issue(issue) end end @@ -69,14 +86,14 @@ describe Issues::CloseService, services: true do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) - described_class.new(project, user).execute(issue) + described_class.new(project, user).close_issue(issue) end end context 'external issue tracker' do before do allow(project).to receive(:default_issues_tracker?).and_return(false) - described_class.new(project, user).execute(issue) + described_class.new(project, user).close_issue(issue) end it { expect(issue).to be_valid } diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb index cf90b33dfb4..57a5aa5cedc 100644 --- a/spec/services/projects/housekeeping_service_spec.rb +++ b/spec/services/projects/housekeeping_service_spec.rb @@ -14,8 +14,10 @@ describe Projects::HousekeepingService do describe '#execute' do it 'enqueues a sidekiq job' do - expect(subject).to receive(:try_obtain_lease).and_return(true) - expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id) + expect(subject).to receive(:try_obtain_lease).and_return(:the_uuid) + expect(subject).to receive(:lease_key).and_return(:the_lease_key) + expect(subject).to receive(:task).and_return(:the_task) + expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :the_task, :the_lease_key, :the_uuid) subject.execute expect(project.reload.pushes_since_gc).to eq(0) @@ -58,4 +60,26 @@ describe Projects::HousekeepingService do end.to change { project.pushes_since_gc }.from(0).to(1) end end + + it 'uses all three kinds of housekeeping we offer' do + allow(subject).to receive(:try_obtain_lease).and_return(:the_uuid) + allow(subject).to receive(:lease_key).and_return(:the_lease_key) + + # At push 200 + expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :gc, :the_lease_key, :the_uuid). + exactly(1).times + # At push 50, 100, 150 + expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :full_repack, :the_lease_key, :the_uuid). + exactly(3).times + # At push 10, 20, ... (except those above) + expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :incremental_repack, :the_lease_key, :the_uuid). + exactly(16).times + + 201.times do + subject.increment! + subject.execute if subject.needed? + end + + expect(project.pushes_since_gc).to eq(1) + end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index c79975d8667..778e665500d 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -204,20 +204,18 @@ module TestEnv end def set_repo_refs(repo_path, branch_sha) + instructions = branch_sha.map {|branch, sha| "update refs/heads/#{branch}\x00#{sha}\x00" }.join("\x00") << "\x00" + update_refs = %W(#{Gitlab.config.git.bin_path} update-ref --stdin -z) + reset = proc do + IO.popen(update_refs, "w") {|io| io.write(instructions) } + $?.success? + end + Dir.chdir(repo_path) do - branch_sha.each do |branch, sha| - # Try to reset without fetching to avoid using the network. - reset = %W(#{Gitlab.config.git.bin_path} update-ref refs/heads/#{branch} #{sha}) - unless system(*reset) - if system(*%W(#{Gitlab.config.git.bin_path} fetch origin)) - unless system(*reset) - raise 'The fetched test seed '\ - 'does not contain the required revision.' - end - else - raise 'Could not fetch test seed repository.' - end - end + # Try to reset without fetching to avoid using the network. + unless reset.call + raise 'Could not fetch test seed repository.' unless system(*%W(#{Gitlab.config.git.bin_path} fetch origin)) + raise 'The fetched test seed does not contain the required revision.' unless reset.call end end end diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index c9f5aae0815..e471a68a49a 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -1,3 +1,5 @@ +require 'fileutils' + require 'spec_helper' describe GitGarbageCollectWorker do @@ -6,16 +8,12 @@ describe GitGarbageCollectWorker do subject { GitGarbageCollectWorker.new } - before do - allow(subject).to receive(:gitlab_shell).and_return(shell) - end - describe "#perform" do - it "runs `git gc`" do - expect(shell).to receive(:gc).with( - project.repository_storage_path, - project.path_with_namespace). - and_return(true) + it "flushes ref caches when the task is 'gc'" do + expect(subject).to receive(:command).with(:gc).and_return([:the, :command]) + expect(Gitlab::Popen).to receive(:popen). + with([:the, :command], project.repository.path_to_repo).and_return(["", 0]) + expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original expect_any_instance_of(Repository).to receive(:branch_count).and_call_original @@ -23,5 +21,110 @@ describe GitGarbageCollectWorker do subject.perform(project.id) end + + shared_examples 'gc tasks' do + before { allow(subject).to receive(:bitmaps_enabled?).and_return(bitmaps_enabled) } + + it 'incremental repack adds a new packfile' do + create_objects(project) + before_packs = packs(project) + + expect(before_packs.count).to be >= 1 + + subject.perform(project.id, 'incremental_repack') + after_packs = packs(project) + + # Exactly one new pack should have been created + expect(after_packs.count).to eq(before_packs.count + 1) + + # Previously existing packs are still around + expect(before_packs & after_packs).to eq(before_packs) + end + + it 'full repack consolidates into 1 packfile' do + create_objects(project) + subject.perform(project.id, 'incremental_repack') + before_packs = packs(project) + + expect(before_packs.count).to be >= 2 + + subject.perform(project.id, 'full_repack') + after_packs = packs(project) + + expect(after_packs.count).to eq(1) + + # Previously existing packs should be gone now + expect(after_packs - before_packs).to eq(after_packs) + + expect(File.exist?(bitmap_path(after_packs.first))).to eq(bitmaps_enabled) + end + + it 'gc consolidates into 1 packfile and updates packed-refs' do + create_objects(project) + before_packs = packs(project) + before_packed_refs = packed_refs(project) + + expect(before_packs.count).to be >= 1 + + subject.perform(project.id, 'gc') + after_packed_refs = packed_refs(project) + after_packs = packs(project) + + expect(after_packs.count).to eq(1) + + # Previously existing packs should be gone now + expect(after_packs - before_packs).to eq(after_packs) + + # The packed-refs file should have been updated during 'git gc' + expect(before_packed_refs).not_to eq(after_packed_refs) + + expect(File.exist?(bitmap_path(after_packs.first))).to eq(bitmaps_enabled) + end + end + + context 'with bitmaps enabled' do + let(:bitmaps_enabled) { true } + + include_examples 'gc tasks' + end + + context 'with bitmaps disabled' do + let(:bitmaps_enabled) { false } + + include_examples 'gc tasks' + end + end + + # Create a new commit on a random new branch + def create_objects(project) + rugged = project.repository.rugged + old_commit = rugged.branches.first.target + new_commit_sha = Rugged::Commit.create( + rugged, + message: "hello world #{SecureRandom.hex(6)}", + author: Gitlab::Git::committer_hash(email: 'foo@bar', name: 'baz'), + committer: Gitlab::Git::committer_hash(email: 'foo@bar', name: 'baz'), + tree: old_commit.tree, + parents: [old_commit], + ) + project.repository.update_ref!( + "refs/heads/#{SecureRandom.hex(6)}", + new_commit_sha, + Gitlab::Git::BLANK_SHA + ) + end + + def packs(project) + Dir["#{project.repository.path_to_repo}/objects/pack/*.pack"] + end + + def packed_refs(project) + path = "#{project.repository.path_to_repo}/packed-refs" + FileUtils.touch(path) + File.read(path) + end + + def bitmap_path(pack) + pack.sub(/\.pack\z/, '.bitmap') end end diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb new file mode 100644 index 00000000000..3e4fee42240 --- /dev/null +++ b/spec/workers/process_commit_worker_spec.rb @@ -0,0 +1,109 @@ +require 'spec_helper' + +describe ProcessCommitWorker do + let(:worker) { described_class.new } + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project, author: user) } + let(:commit) { project.commit } + + describe '#perform' do + it 'does not process the commit when the project does not exist' do + expect(worker).not_to receive(:close_issues) + + worker.perform(-1, user.id, commit.id) + end + + it 'does not process the commit when the user does not exist' do + expect(worker).not_to receive(:close_issues) + + worker.perform(project.id, -1, commit.id) + end + + it 'does not process the commit when the commit no longer exists' do + expect(worker).not_to receive(:close_issues) + + worker.perform(project.id, user.id, 'this-should-does-not-exist') + end + + it 'processes the commit message' do + expect(worker).to receive(:process_commit_message).and_call_original + + worker.perform(project.id, user.id, commit.id) + end + + it 'updates the issue metrics' do + expect(worker).to receive(:update_issue_metrics).and_call_original + + worker.perform(project.id, user.id, commit.id) + end + end + + describe '#process_commit_message' do + context 'when pushing to the default branch' do + it 'closes issues that should be closed per the commit message' do + allow(commit).to receive(:safe_message). + and_return("Closes #{issue.to_reference}") + + expect(worker).to receive(:close_issues). + with(project, user, user, commit, [issue]) + + worker.process_commit_message(project, commit, user, user, true) + end + end + + context 'when pushing to a non-default branch' do + it 'does not close any issues' do + allow(commit).to receive(:safe_message). + and_return("Closes #{issue.to_reference}") + + expect(worker).not_to receive(:close_issues) + + worker.process_commit_message(project, commit, user, user, false) + end + end + + it 'creates cross references' do + expect(commit).to receive(:create_cross_references!) + + worker.process_commit_message(project, commit, user, user) + end + end + + describe '#close_issues' do + context 'when the user can update the issues' do + it 'closes the issues' do + worker.close_issues(project, user, user, commit, [issue]) + + issue.reload + + expect(issue.closed?).to eq(true) + end + end + + context 'when the user can not update the issues' do + it 'does not close the issues' do + other_user = create(:user) + + worker.close_issues(project, other_user, other_user, commit, [issue]) + + issue.reload + + expect(issue.closed?).to eq(false) + end + end + end + + describe '#update_issue_metrics' do + it 'updates any existing issue metrics' do + allow(commit).to receive(:safe_message). + and_return("Closes #{issue.to_reference}") + + worker.update_issue_metrics(commit, user) + + metric = Issue::Metrics.first + + expect(metric.first_mentioned_in_commit_at).to eq(commit.committed_date) + end + end +end |