diff options
35 files changed, 334 insertions, 195 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dcdf520ee6b..5a66e04247e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,25 +27,26 @@ _This notice should stay as the first item in the CONTRIBUTING.md file._ - [Helping others](#helping-others) - [I want to contribute!](#i-want-to-contribute) - [Workflow labels](#workflow-labels) - - [Type labels (~"feature proposal", ~bug, ~customer, etc.)](#type-labels-feature-proposal-bug-customer-etc) - - [Subject labels (~wiki, ~"container registry", ~ldap, ~api, etc.)](#subject-labels-wiki-container-registry-ldap-api-etc) - - [Team labels (~"CI/CD", ~Discussion, ~Quality, ~Platform, etc.)](#team-labels-cicd-discussion-quality-platform-etc) - - [Milestone labels (~Deliverable, ~Stretch, ~"Next Patch Release")](#milestone-labels-deliverable-stretch-next-patch-release) - - [Priority labels (~P1, ~P2, ~P3 , ~P4)](#bug-priority-labels-p1-p2-p3-p4) - - [Severity labels (~S1, ~S2, ~S3 , ~S4)](#bug-severity-labels-s1-s2-s3-s4) - - [Label for community contributors (~"Accepting Merge Requests")](#label-for-community-contributors-accepting-merge-requests) -- [Implement design & UI elements](#implement-design--ui-elements) + - [Type labels](#type-labels) + - [Subject labels](#subject-labels) + - [Team labels](#team-labels) + - [Milestone labels](#milestone-labels) + - [Bug Priority labels](#bug-priority-labels) + - [Bug Severity labels](#bug-severity-labels) + - [Severity impact guidance](#severity-impact-guidance) + - [Label for community contributors](#label-for-community-contributors) +- [Implement design & UI elements](#implement-design-ui-elements) - [Issue tracker](#issue-tracker) - - [Issue triaging](#issue-triaging) - - [Feature proposals](#feature-proposals) - - [Issue tracker guidelines](#issue-tracker-guidelines) - - [Issue weight](#issue-weight) - - [Regression issues](#regression-issues) - - [Technical and UX debt](#technical-and-ux-debt) - - [Stewardship](#stewardship) + - [Issue triaging](#issue-triaging) + - [Feature proposals](#feature-proposals) + - [Issue tracker guidelines](#issue-tracker-guidelines) + - [Issue weight](#issue-weight) + - [Regression issues](#regression-issues) + - [Technical and UX debt](#technical-and-ux-debt) + - [Stewardship](#stewardship) - [Merge requests](#merge-requests) - - [Merge request guidelines](#merge-request-guidelines) - - [Contribution acceptance criteria](#contribution-acceptance-criteria) + - [Merge request guidelines](#merge-request-guidelines) + - [Contribution acceptance criteria](#contribution-acceptance-criteria) - [Definition of done](#definition-of-done) - [Style guides](#style-guides) - [Code of conduct](#code-of-conduct) @@ -145,7 +146,7 @@ labels, you can _always_ add the team and type, and often also the subject. [milestones-page]: https://gitlab.com/gitlab-org/gitlab-ce/milestones [labels-page]: https://gitlab.com/gitlab-org/gitlab-ce/labels -### Type labels (~"feature proposal", ~bug, ~customer, etc.) +### Type labels Type labels are very important. They define what kind of issue this is. Every issue should have one or more. @@ -161,28 +162,41 @@ already reserved for subject labels). The descriptions on the [labels page][labels-page] explain what falls under each type label. -### Subject labels (~wiki, ~"container registry", ~ldap, ~api, etc.) +### Subject labels Subject labels are labels that define what area or feature of GitLab this issue hits. They are not always necessary, but very convenient. +Examples of subject labels are ~wiki, ~ldap, ~api, +~issues, ~"merge requests", ~labels, and ~"container registry". + If you are an expert in a particular area, it makes it easier to find issues to work on. You can also subscribe to those labels to receive an email each time an issue is labeled with a subject label corresponding to your expertise. -Examples of subject labels are ~wiki, ~"container registry", ~ldap, ~api, -~issues, ~"merge requests", ~labels, and ~"container registry". - Subject labels are always all-lowercase. -### Team labels (~"CI/CD", ~Discussion, ~Quality, ~Platform, etc.) +### Team labels Team labels specify what team is responsible for this issue. Assigning a team label makes sure issues get the attention of the appropriate people. -The current team labels are ~Distribution, ~"CI/CD", ~Discussion, ~Documentation, ~Quality, -~Geo, ~Gitaly, ~Monitoring, ~Platform, ~Release, ~"Security Products", ~"Configuration", and ~"UX". +The current team labels are: + +- ~Configuration +- ~"CI/CD" +- ~Discussion +- ~Distribution +- ~Documentation +- ~Geo +- ~Gitaly +- ~Monitoring +- ~Platform +- ~Quality +- ~Release +- ~"Security Products" +- ~UX The descriptions on the [labels page][labels-page] explain what falls under the responsibility of each team. @@ -193,7 +207,7 @@ indicate if an issue needs backend work, frontend work, or both. Team labels are always capitalized so that they show up as the first label for any issue. -### Milestone labels (~Deliverable, ~Stretch, ~"Next Patch Release") +### Milestone labels Milestone labels help us clearly communicate expectations of the work for the release. There are three levels of Milestone labels: @@ -211,9 +225,9 @@ Each issue scheduled for the current milestone should be labeled ~Deliverable or ~"Stretch". Any open issue for a previous milestone should be labeled ~"Next Patch Release", or otherwise rescheduled to a different milestone. -### Bug Priority labels (~P1, ~P2, ~P3, ~P4) +### Bug Priority labels -Bug Priority labels help us define the time a ~bug fix should be completed. Priority determines how quickly the defect turnaround time must be. +Bug Priority labels help us define the time a ~bug fix should be completed. Priority determines how quickly the defect turnaround time must be. If there are multiple defects, the priority decides which defect has to be fixed immediately versus later. This label documents the planned timeline & urgency which is used to measure against our actual SLA on delivering ~bug fixes. @@ -224,7 +238,7 @@ This label documents the planned timeline & urgency which is used to measure aga | ~P3 | Medium Priority | Within the next 3 releases (approx one quarter) | | | ~P4 | Low Priority | Anything outside the next 3 releases (approx beyond one quarter) | The issue is prominent but does not impact user workflow and a workaround is documented | -### Bug Severity labels (~S1, ~S2, ~S3, ~S4) +### Bug Severity labels Severity labels help us clearly communicate the impact of a ~bug on users. @@ -240,11 +254,11 @@ Severity labels help us clearly communicate the impact of a ~bug on users. | Label | Security Impact | Availability / Performance Impact | |-------|---------------------------------------------------------------------|--------------------------------------------------------------| | ~S1 | >50% users impacted (possible company extinction level event) | | -| ~S2 | Many users or multiple paid customers impacted (but not apocalyptic)| The issue is (almost) guaranteed to occur in the near future | +| ~S2 | Many users or multiple paid customers impacted (but not apocalyptic)| The issue is (almost) guaranteed to occur in the near future | | ~S3 | A few users or a single paid customer impacted | The issue is likely to occur in the near future | | ~S4 | No paid users/customer impacted, or expected impact within 30 days | The issue _may_ occur but it's not likely | -### Label for community contributors (~"Accepting Merge Requests") +### Label for community contributors Issues that are beneficial to our users, 'nice to haves', that we currently do not have the capacity for or want to give the priority to, are labeled as @@ -300,14 +314,14 @@ For guidance on UX implementation at GitLab, please refer to our [Design System] The UX team uses labels to manage their workflow. -The ~"UX" label on an issue is a signal to the UX team that it will need UX attention. +The ~"UX" label on an issue is a signal to the UX team that it will need UX attention. To better understand the priority by which UX tackles issues, see the [UX section](https://about.gitlab.com/handbook/engineering/ux) of the handbook. Once an issue has been worked on and is ready for development, a UXer removes the ~"UX" label and applies the ~"UX ready" label to that issue. -The UX team has a special type label called ~"design artifact". This label indicates that the final output +The UX team has a special type label called ~"design artifact". This label indicates that the final output for an issue is a UX solution/design. The solution will be developed by frontend and/or backend in a subsequent milestone. -Any issue labeled ~"design artifact" should not also be labeled ~"frontend" or ~"backend" since no development is +Any issue labeled ~"design artifact" should not also be labeled ~"frontend" or ~"backend" since no development is needed until the solution has been decided. ~"design artifact" issues are like any other issue and should contain a milestone label, ~"Deliverable" or ~"Stretch", when scheduled in the current milestone. diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index 170bca8b56f..16374146ae4 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -1,9 +1,15 @@ module UploadsActions + extend ActiveSupport::Concern + include Gitlab::Utils::StrongMemoize include SendFileUpload UPLOAD_MOUNTS = %w(avatar attachment file logo header_logo favicon).freeze + included do + prepend_before_action :set_html_format, only: :show + end + def create link_to_file = UploadService.new(model, params[:file], uploader_class).execute @@ -41,6 +47,13 @@ module UploadsActions private + # Explicitly set the format. + # Otherwise rails 5 will set it from a file extension. + # See https://github.com/rails/rails/commit/84e8accd6fb83031e4c27e44925d7596655285f7#diff-2b8f2fbb113b55ca8e16001c393da8f1 + def set_html_format + request.format = :html + end + def uploader_class raise NotImplementedError end diff --git a/app/models/project.rb b/app/models/project.rb index e5fa1c4db7b..0d777515536 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -25,6 +25,7 @@ class Project < ActiveRecord::Base include FastDestroyAll::Helpers include WithUploads include BatchDestroyDependentAssociations + extend Gitlab::Cache::RequestCache extend Gitlab::ConfigHelper @@ -2013,6 +2014,11 @@ class Project < ActiveRecord::Base @gitlab_deploy_token ||= deploy_tokens.gitlab_deploy_token end + def any_lfs_file_locks? + lfs_file_locks.any? + end + request_cache(:any_lfs_file_locks?) { self.id } + private def storage diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index ae0debbd3ac..a60b4c7fd0d 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -155,6 +155,7 @@ class ChatNotificationService < Service end def notify_for_ref?(data) + return true if data[:object_kind] == 'tag_push' return true if data.dig(:object_attributes, :tag) return true unless notify_only_default_branch? diff --git a/app/views/email_rejection_mailer/rejection.html.haml b/app/views/email_rejection_mailer/rejection.html.haml index 7f7d841fe21..c4ae7befe4e 100644 --- a/app/views/email_rejection_mailer/rejection.html.haml +++ b/app/views/email_rejection_mailer/rejection.html.haml @@ -2,3 +2,4 @@ Unfortunately, your email message to GitLab could not be processed. = markdown @reason += render_if_exists 'shared/additional_email_text' diff --git a/app/views/email_rejection_mailer/rejection.text.haml b/app/views/email_rejection_mailer/rejection.text.haml index af518b5b583..0e13b2a6473 100644 --- a/app/views/email_rejection_mailer/rejection.text.haml +++ b/app/views/email_rejection_mailer/rejection.text.haml @@ -1,3 +1,4 @@ Unfortunately, your email message to GitLab could not be processed. \ = @reason += render_if_exists 'shared/additional_email_text' diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 0a9adc6f243..dd6a84e503d 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -9,6 +9,8 @@ %p Assignee: #{@merge_request.assignee_name} += render_if_exists 'notify/merge_request_approvers', merge_request: @merge_request + - if @merge_request.description %div = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author) diff --git a/app/views/notify/new_merge_request_email.text.erb b/app/views/notify/new_merge_request_email.text.erb index 7d98400e6fe..d5b8f8d764f 100644 --- a/app/views/notify/new_merge_request_email.text.erb +++ b/app/views/notify/new_merge_request_email.text.erb @@ -5,6 +5,6 @@ New Merge Request <%= @merge_request.to_reference %> <%= merge_path_description(@merge_request, 'to') %> Author: <%= @merge_request.author_name %> Assignee: <%= @merge_request.assignee_name %> +<%= render_if_exists 'notify/merge_request_approvers', merge_request: @merge_request %> <%= @merge_request.description %> - diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml index 1b150ec3e5c..0a0b3ce1d6f 100644 --- a/app/views/projects/blob/_header.html.haml +++ b/app/views/projects/blob/_header.html.haml @@ -11,6 +11,7 @@ = view_on_environment_button(@commit.sha, @path, @environment) if @environment .btn-group{ role: "group" }< + = render_if_exists 'projects/blob/header_file_locks_link' = edit_blob_button = ide_edit_button - if current_user @@ -18,3 +19,4 @@ = delete_blob_link = render 'projects/fork_suggestion' += render_if_exists 'projects/blob/header_file_locks', project: @project, path: @path diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index b478fbbb15e..f7b04c436a6 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -69,6 +69,8 @@ .wiki = markdown_field(@milestone, :description) + = render_if_exists 'shared/milestones/burndown', milestone: @milestone, project: @project + - if can?(current_user, :read_issue, @project) && @milestone.total_items_count(current_user).zero? .alert.alert-success.prepend-top-default %span Assign some issues to this milestone. diff --git a/app/views/projects/refs/logs_tree.js.haml b/app/views/projects/refs/logs_tree.js.haml index d07bb661615..506bf54b3f8 100644 --- a/app/views/projects/refs/logs_tree.js.haml +++ b/app/views/projects/refs/logs_tree.js.haml @@ -8,6 +8,8 @@ row.find("td.tree-time-ago").html('#{escape_javascript time_ago_with_tooltip(commit.committed_date)}'); row.find("td.tree-commit").html('#{escape_javascript render("projects/tree/tree_commit_column", commit: commit)}'); + = render_if_exists 'projects/refs/logs_tree_lock_label', lock_label: content_data[:lock_label] + - if @more_log_url :plain if($('#tree-slider').length) { diff --git a/changelogs/unreleased/45487-slack-tag-push-notifs.yml b/changelogs/unreleased/45487-slack-tag-push-notifs.yml new file mode 100644 index 00000000000..647000bd97c --- /dev/null +++ b/changelogs/unreleased/45487-slack-tag-push-notifs.yml @@ -0,0 +1,5 @@ +--- +title: Fix chat service tag notifications not sending when only default branch enabled +merge_request: 19864 +author: +type: fixed diff --git a/changelogs/unreleased/fix-groups-api-ordering.yml b/changelogs/unreleased/fix-groups-api-ordering.yml new file mode 100644 index 00000000000..3a6a7f84356 --- /dev/null +++ b/changelogs/unreleased/fix-groups-api-ordering.yml @@ -0,0 +1,4 @@ +title: Fixed pagination of groups API +merge_request: 19665 +author: Marko, Peter +type: added diff --git a/changelogs/unreleased/more-group-api-sorting-options.yml b/changelogs/unreleased/more-group-api-sorting-options.yml new file mode 100644 index 00000000000..b29f76a65a9 --- /dev/null +++ b/changelogs/unreleased/more-group-api-sorting-options.yml @@ -0,0 +1,5 @@ +--- +title: Added id sorting option to GET groups and subgroups API +merge_request: 19665 +author: Marko, Peter +type: added diff --git a/changelogs/unreleased/rails5-fix-46276.yml b/changelogs/unreleased/rails5-fix-46276.yml new file mode 100644 index 00000000000..cdca91a755d --- /dev/null +++ b/changelogs/unreleased/rails5-fix-46276.yml @@ -0,0 +1,5 @@ +--- +title: Rails5 fix format in uploads actions +merge_request: 19907 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/sh-optimize-locks-check-ce.yml b/changelogs/unreleased/sh-optimize-locks-check-ce.yml new file mode 100644 index 00000000000..933ec9b79bf --- /dev/null +++ b/changelogs/unreleased/sh-optimize-locks-check-ce.yml @@ -0,0 +1,5 @@ +--- +title: Eliminate N+1 queries in LFS file locks checks during a push +merge_request: +author: +type: performance diff --git a/doc/administration/auth/how_to_configure_ldap_gitlab_ce/index.md b/doc/administration/auth/how_to_configure_ldap_gitlab_ce/index.md index aa5e9513290..621d4f77d5e 100644 --- a/doc/administration/auth/how_to_configure_ldap_gitlab_ce/index.md +++ b/doc/administration/auth/how_to_configure_ldap_gitlab_ce/index.md @@ -107,7 +107,7 @@ Global Admins GitLab.org/GitLab INT/Global Groups/Global Admins ## GitLab LDAP configuration -The initial configuration of LDAP in GitLab requires changes to the `gitlab.rb` configuration file. Below is an example of a complete configuration using an Active Directory. +The initial configuration of LDAP in GitLab requires changes to the `gitlab.rb` configuration file (`/etc/gitlab/gitlab.rb`). Below is an example of a complete configuration using an Active Directory. The two Active Directory specific values are `active_directory: true` and `uid: 'sAMAccountName'`. `sAMAccountName` is an attribute returned by Active Directory used for GitLab usernames. See the example output from `ldapsearch` for a full list of attributes a "person" object (user) has in **AD** - [`ldapsearch` example](#using-ldapsearch-unix) diff --git a/doc/api/groups.md b/doc/api/groups.md index 96842ef330f..a48905f2f15 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -12,7 +12,7 @@ Parameters: | `skip_groups` | array of integers | no | Skip the group IDs passed | | `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for admin) | | `search` | string | no | Return the list of authorized groups matching the search criteria | -| `order_by` | string | no | Order groups by `name` or `path`. Default is `name` | +| `order_by` | string | no | Order groups by `name`, `path` or `id`. Default is `name` | | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` | | `statistics` | boolean | no | Include group statistics (admins only) | | `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | @@ -96,7 +96,7 @@ Parameters: | `skip_groups` | array of integers | no | Skip the group IDs passed | | `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for admin) | | `search` | string | no | Return the list of authorized groups matching the search criteria | -| `order_by` | string | no | Order groups by `name` or `path`. Default is `name` | +| `order_by` | string | no | Order groups by `name`, `path` or `id`. Default is `name` | | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` | | `statistics` | boolean | no | Include group statistics (admins only) | | `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index 3edde3de83d..82e8fbdb93e 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -168,7 +168,7 @@ want their accounts to be upgraded to full internal accounts. >**Note:** The following information only applies for installations from source. -GitLab uses [Omniauth](http://www.omniauth.org/) for authentication and already ships +GitLab uses [Omniauth](https://github.com/omniauth/omniauth) for authentication and already ships with a few providers pre-installed (e.g. LDAP, GitHub, Twitter). But sometimes that is not enough and you need to integrate with other authentication solutions. For these cases you can use the Omniauth provider. diff --git a/doc/user/project/repository/index.md b/doc/user/project/repository/index.md index 376f4e3cbe4..bda293bd00e 100644 --- a/doc/user/project/repository/index.md +++ b/doc/user/project/repository/index.md @@ -176,4 +176,12 @@ Lock your files to prevent any conflicting changes. You can access your repos via [repository API](../../../api/repositories.md). +## Clone in Apple Xcode + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/45820) in GitLab 11.0 + +Projects that contain a `.xcodeproj` or `.xcworkspace` directory can now be cloned +in Xcode using the new **Open in Xcode** button, located next to the Git URL +used for cloning your project. The button is only shown on macOS. + [jupyter]: https://jupyter.org diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 03b6b30a0d8..c7f41aba854 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -32,7 +32,7 @@ module API optional :all_available, type: Boolean, desc: 'Show all group that you have access to' optional :search, type: String, desc: 'Search for a specific group' optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user' - optional :order_by, type: String, values: %w[name path], default: 'name', desc: 'Order by name or path' + optional :order_by, type: String, values: %w[name path id], default: 'name', desc: 'Order by name, path or id' optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)' use :pagination end @@ -46,7 +46,9 @@ module API groups = GroupsFinder.new(current_user, find_params).execute groups = groups.search(params[:search]) if params[:search].present? groups = groups.where.not(id: params[:skip_groups]) if params[:skip_groups].present? - groups = groups.reorder(params[:order_by] => params[:sort]) + order_options = { params[:order_by] => params[:sort] } + order_options["id"] ||= "asc" + groups = groups.reorder(order_options) groups end diff --git a/lib/gitlab/checks/commit_check.rb b/lib/gitlab/checks/commit_check.rb index 43a52b493bb..22310e313ac 100644 --- a/lib/gitlab/checks/commit_check.rb +++ b/lib/gitlab/checks/commit_check.rb @@ -37,7 +37,7 @@ module Gitlab def validate_lfs_file_locks? strong_memoize(:validate_lfs_file_locks) do - project.lfs_enabled? && project.lfs_file_locks.any? && newrev && oldrev + project.lfs_enabled? && newrev && oldrev && project.any_lfs_file_locks? end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 7e90bca0a1f..ab0e6e33db7 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -684,15 +684,9 @@ module Gitlab end # Return total commits count accessible from passed ref - # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/330 def commit_count(ref) - gitaly_migrate(:commit_count, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_commit_client.commit_count(ref) - else - rugged_commit_count(ref) - end + wrapped_gitaly_errors do + gitaly_commit_client.commit_count(ref) end end @@ -992,21 +986,7 @@ module Gitlab def info_attributes return @info_attributes if @info_attributes - content = - gitaly_migrate(:get_info_attributes, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_repository_client.info_attributes - else - attributes_path = File.join(File.expand_path(path), 'info', 'attributes') - - if File.exist?(attributes_path) - File.read(attributes_path) - else - "" - end - end - end - + content = gitaly_repository_client.info_attributes @info_attributes = AttributesParser.new(content) end @@ -1062,18 +1042,8 @@ module Gitlab end def license_short_name - gitaly_migrate(:license_short_name, - status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_repository_client.license_short_name - else - begin - # The licensee gem creates a Rugged object from the path: - # https://github.com/benbalter/licensee/blob/v8.7.0/lib/licensee/projects/git_project.rb - Licensee.license(path).try(:key) - rescue Rugged::Error - end - end + wrapped_gitaly_errors do + gitaly_repository_client.license_short_name end end @@ -1260,12 +1230,8 @@ module Gitlab end def rebase_in_progress?(rebase_id) - gitaly_migrate(:rebase_in_progress) do |is_enabled| - if is_enabled - gitaly_repository_client.rebase_in_progress?(rebase_id) - else - fresh_worktree?(worktree_path(REBASE_WORKTREE_PREFIX, rebase_id)) - end + wrapped_gitaly_errors do + gitaly_repository_client.rebase_in_progress?(rebase_id) end end @@ -1281,12 +1247,8 @@ module Gitlab end def squash_in_progress?(squash_id) - gitaly_migrate(:squash_in_progress, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_repository_client.squash_in_progress?(squash_id) - else - fresh_worktree?(worktree_path(SQUASH_WORKTREE_PREFIX, squash_id)) - end + wrapped_gitaly_errors do + gitaly_repository_client.squash_in_progress?(squash_id) end end @@ -1629,21 +1591,6 @@ module Gitlab end end - # This function is duplicated in Gitaly-Go, don't change it! - # https://gitlab.com/gitlab-org/gitaly/merge_requests/698 - def fresh_worktree?(path) - File.exist?(path) && !clean_stuck_worktree(path) - end - - # This function is duplicated in Gitaly-Go, don't change it! - # https://gitlab.com/gitlab-org/gitaly/merge_requests/698 - def clean_stuck_worktree(path) - return false unless File.mtime(path) < 15.minutes.ago - - FileUtils.rm_rf(path) - true - end - # Adding a worktree means checking out the repository. For large repos, # this can be very expensive, so set up sparse checkout for the worktree # to only check out the files we're interested in. @@ -2336,16 +2283,6 @@ module Gitlab nil end - def rugged_commit_count(ref) - walker = Rugged::Walker.new(rugged) - walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE) - oid = rugged.rev_parse_oid(ref) - walker.push(oid) - walker.count - rescue Rugged::ReferenceError - 0 - end - def rev_list_param(spec) spec == :all ? ['--all'] : spec end diff --git a/spec/lib/gitlab/auth/user_auth_finders_spec.rb b/spec/lib/gitlab/auth/user_auth_finders_spec.rb index 136646bd4ee..454ad1589b9 100644 --- a/spec/lib/gitlab/auth/user_auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/user_auth_finders_spec.rb @@ -99,7 +99,7 @@ describe Gitlab::Auth::UserAuthFinders do context 'when the request format is empty' do it 'the method call does not modify the original value' do - env['action_dispatch.request.formats'] = nil + env.delete('action_dispatch.request.formats') find_user_from_feed_token diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index bc5a5e43103..2e204da307d 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -49,7 +49,7 @@ describe Gitlab::Ci::Config do describe '.new' do it 'raises error' do expect { config }.to raise_error( - Gitlab::Ci::Config::Loader::FormatError, + ::Gitlab::Ci::Config::Loader::FormatError, /Invalid configuration format/ ) end diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index c5a4d9b4778..284aed91e29 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Chain::Populate do - set(:project) { create(:project) } + set(:project) { create(:project, :repository) } set(:user) { create(:user) } let(:pipeline) do @@ -174,7 +174,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do end let(:pipeline) do - build(:ci_pipeline, ref: 'master', config: config) + build(:ci_pipeline, ref: 'master', project: project, config: config) end it_behaves_like 'a correct pipeline' diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index c53294d091c..a8dc5356413 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Chain::Validate::Config do - set(:project) { create(:project) } + set(:project) { create(:project, :repository) } set(:user) { create(:user) } let(:command) do diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index ecb16daec96..fa5327c26f0 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' module Gitlab module Ci - describe YamlProcessor, :lib do + describe YamlProcessor do subject { described_class.new(config) } describe 'our current .gitlab-ci.yml' do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index b9eb002df3b..595482f76d5 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1680,70 +1680,52 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#languages' do - shared_examples 'languages' do - it 'returns exactly the expected results' do - languages = repository.languages('4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6') - expected_languages = [ - { value: 66.63, label: "Ruby", color: "#701516", highlight: "#701516" }, - { value: 22.96, label: "JavaScript", color: "#f1e05a", highlight: "#f1e05a" }, - { value: 7.9, label: "HTML", color: "#e34c26", highlight: "#e34c26" }, - { value: 2.51, label: "CoffeeScript", color: "#244776", highlight: "#244776" } - ] + it 'returns exactly the expected results' do + languages = repository.languages('4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6') + expected_languages = [ + { value: 66.63, label: "Ruby", color: "#701516", highlight: "#701516" }, + { value: 22.96, label: "JavaScript", color: "#f1e05a", highlight: "#f1e05a" }, + { value: 7.9, label: "HTML", color: "#e34c26", highlight: "#e34c26" }, + { value: 2.51, label: "CoffeeScript", color: "#244776", highlight: "#244776" } + ] - expect(languages.size).to eq(expected_languages.size) + expect(languages.size).to eq(expected_languages.size) - expected_languages.size.times do |i| - a = expected_languages[i] - b = languages[i] + expected_languages.size.times do |i| + a = expected_languages[i] + b = languages[i] - expect(a.keys.sort).to eq(b.keys.sort) - expect(a[:value]).to be_within(0.1).of(b[:value]) + expect(a.keys.sort).to eq(b.keys.sort) + expect(a[:value]).to be_within(0.1).of(b[:value]) - non_float_keys = a.keys - [:value] - expect(a.values_at(*non_float_keys)).to eq(b.values_at(*non_float_keys)) - end - end - - it "uses the repository's HEAD when no ref is passed" do - lang = repository.languages.first - - expect(lang[:label]).to eq('Ruby') + non_float_keys = a.keys - [:value] + expect(a.values_at(*non_float_keys)).to eq(b.values_at(*non_float_keys)) end end - it_behaves_like 'languages' + it "uses the repository's HEAD when no ref is passed" do + lang = repository.languages.first - context 'with rugged', :skip_gitaly_mock do - it_behaves_like 'languages' + expect(lang[:label]).to eq('Ruby') end end describe '#license_short_name' do - shared_examples 'acquiring the Licensee license key' do - subject { repository.license_short_name } - - context 'when no license file can be found' do - let(:project) { create(:project, :repository) } - let(:repository) { project.repository.raw_repository } - - before do - project.repository.delete_file(project.owner, 'LICENSE', message: 'remove license', branch_name: 'master') - end + subject { repository.license_short_name } - it { is_expected.to be_nil } - end + context 'when no license file can be found' do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository.raw_repository } - context 'when an mit license is found' do - it { is_expected.to eq('mit') } + before do + project.repository.delete_file(project.owner, 'LICENSE', message: 'remove license', branch_name: 'master') end - end - context 'when gitaly is enabled' do - it_behaves_like 'acquiring the Licensee license key' + it { is_expected.to be_nil } end - context 'when gitaly is disabled', :disable_gitaly do - it_behaves_like 'acquiring the Licensee license key' + context 'when an mit license is found' do + it { is_expected.to eq('mit') } end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 0d5f6a0b576..ff32025253a 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -934,6 +934,22 @@ describe Gitlab::GitAccess do expect(project.repository).to receive(:clean_stale_repository_files).and_call_original expect { push_access_check }.not_to raise_error end + + it 'avoids N+1 queries', :request_store do + # Run this once to establish a baseline. Cached queries should get + # cached, so that when we introduce another change we shouldn't see + # additional queries. + access.check('git-receive-pack', changes) + + control_count = ActiveRecord::QueryRecorder.new do + access.check('git-receive-pack', changes) + end + + changes = ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'] + + # There is still an N+1 query with protected branches + expect { access.check('git-receive-pack', changes) }.not_to exceed_query_limit(control_count).with_threshold(1) + end end end diff --git a/spec/migrations/remove_soft_removed_objects_spec.rb b/spec/migrations/remove_soft_removed_objects_spec.rb index fb70c284f5e..d0bde98b80e 100644 --- a/spec/migrations/remove_soft_removed_objects_spec.rb +++ b/spec/migrations/remove_soft_removed_objects_spec.rb @@ -3,6 +3,18 @@ require Rails.root.join('db', 'post_migrate', '20171207150343_remove_soft_remove describe RemoveSoftRemovedObjects, :migration do describe '#up' do + let!(:groups) do + table(:namespaces).tap do |t| + t.inheritance_column = nil + end + end + + let!(:routes) do + table(:routes).tap do |t| + t.inheritance_column = nil + end + end + it 'removes various soft removed objects' do 5.times do create_with_deleted_at(:issue) @@ -28,19 +40,20 @@ describe RemoveSoftRemovedObjects, :migration do it 'removes routes of soft removed personal namespaces' do namespace = create_with_deleted_at(:namespace) - group = create(:group) # rubocop:disable RSpec/FactoriesInMigrationSpecs + group = groups.create!(name: 'group', path: 'group_path', type: 'Group') + routes.create!(source_id: group.id, source_type: 'Group', name: 'group', path: 'group_path') - expect(Route.where(source: namespace).exists?).to eq(true) - expect(Route.where(source: group).exists?).to eq(true) + expect(routes.where(source_id: namespace.id).exists?).to eq(true) + expect(routes.where(source_id: group.id).exists?).to eq(true) run_migration - expect(Route.where(source: namespace).exists?).to eq(false) - expect(Route.where(source: group).exists?).to eq(true) + expect(routes.where(source_id: namespace.id).exists?).to eq(false) + expect(routes.where(source_id: group.id).exists?).to eq(true) end it 'schedules the removal of soft removed groups' do - group = create_with_deleted_at(:group) + group = create_deleted_group admin = create(:user, admin: true) # rubocop:disable RSpec/FactoriesInMigrationSpecs expect_any_instance_of(GroupDestroyWorker) @@ -51,7 +64,7 @@ describe RemoveSoftRemovedObjects, :migration do end it 'does not remove soft removed groups when no admin user could be found' do - create_with_deleted_at(:group) + create_deleted_group expect_any_instance_of(GroupDestroyWorker) .not_to receive(:perform) @@ -74,4 +87,13 @@ describe RemoveSoftRemovedObjects, :migration do row end + + def create_deleted_group + group = groups.create!(name: 'group', path: 'group_path', type: 'Group') + routes.create!(source_id: group.id, source_type: 'Group', name: 'group', path: 'group_path') + + groups.where(id: group.id).update_all(deleted_at: 1.year.ago) + + group + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index bc9cce6b0c3..a2f8fac2f38 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2339,6 +2339,22 @@ describe Project do end end + describe '#any_lfs_file_locks?', :request_store do + set(:project) { create(:project) } + + it 'returns false when there are no LFS file locks' do + expect(project.any_lfs_file_locks?).to be_falsey + end + + it 'returns a cached true when there are LFS file locks' do + create(:lfs_file_lock, project: project) + + expect(project.lfs_file_locks).to receive(:any?).once.and_call_original + + 2.times { expect(project.any_lfs_file_locks?).to be_truthy } + end + end + describe '#protected_for?' do let(:project) { create(:project) } diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 7d923932309..da23fdd7dca 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -138,10 +138,15 @@ describe API::Groups do context "when using sorting" do let(:group3) { create(:group, name: "a#{group1.name}", path: "z#{group1.path}") } + let(:group4) { create(:group, name: "same-name", path: "y#{group1.path}") } + let(:group5) { create(:group, name: "same-name") } let(:response_groups) { json_response.map { |group| group['name'] } } + let(:response_groups_ids) { json_response.map { |group| group['id'] } } before do group3.add_owner(user1) + group4.add_owner(user1) + group5.add_owner(user1) end it "sorts by name ascending by default" do @@ -150,7 +155,7 @@ describe API::Groups do expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(response_groups).to eq([group3.name, group1.name]) + expect(response_groups).to eq(Group.visible_to_user(user1).order(:name).pluck(:name)) end it "sorts in descending order when passed" do @@ -159,16 +164,52 @@ describe API::Groups do expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(response_groups).to eq([group1.name, group3.name]) + expect(response_groups).to eq(Group.visible_to_user(user1).order(name: :desc).pluck(:name)) end - it "sorts by the order_by param" do + it "sorts by path in order_by param" do get api("/groups", user1), order_by: "path" expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(response_groups).to eq([group1.name, group3.name]) + expect(response_groups).to eq(Group.visible_to_user(user1).order(:path).pluck(:name)) + end + + it "sorts by id in the order_by param" do + get api("/groups", user1), order_by: "id" + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(response_groups).to eq(Group.visible_to_user(user1).order(:id).pluck(:name)) + end + + it "sorts also by descending id with pagination fix" do + get api("/groups", user1), order_by: "id", sort: "desc" + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(response_groups).to eq(Group.visible_to_user(user1).order(id: :desc).pluck(:name)) + end + + it "sorts identical keys by id for good pagination" do + get api("/groups", user1), search: "same-name", order_by: "name" + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(response_groups_ids).to eq(Group.select { |group| group['name'] == 'same-name' }.map { |group| group['id'] }.sort) + end + + it "sorts descending identical keys by id for good pagination" do + get api("/groups", user1), search: "same-name", order_by: "name", sort: "desc" + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(response_groups_ids).to eq(Group.select { |group| group['name'] == 'same-name' }.map { |group| group['id'] }.sort) end end diff --git a/spec/support/helpers/features/notes_helpers.rb b/spec/support/helpers/features/notes_helpers.rb index 1a1d5853a7a..2b9f8b30c60 100644 --- a/spec/support/helpers/features/notes_helpers.rb +++ b/spec/support/helpers/features/notes_helpers.rb @@ -13,7 +13,7 @@ module Spec module Features module NotesHelpers def add_note(text) - Sidekiq::Testing.fake! do + perform_enqueued_jobs do page.within(".js-main-target-form") do fill_in("note[note]", with: text) find(".js-comment-submit-button").click diff --git a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb index 2228e872926..7c34c7b4977 100644 --- a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb @@ -245,6 +245,70 @@ RSpec.shared_examples 'slack or mattermost notifications' do end end + describe 'Push events' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository, creator: user) } + + before do + allow(chat_service).to receive_messages( + project: project, + service_hook: true, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url) + end + + context 'only notify for the default branch' do + context 'when enabled' do + before do + chat_service.notify_only_default_branch = true + end + + it 'does not notify push events if they are not for the default branch' do + ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test" + push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, []) + + chat_service.execute(push_sample_data) + + expect(WebMock).not_to have_requested(:post, webhook_url) + end + + it 'notifies about push events for the default branch' do + push_sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) + + chat_service.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + + it 'still notifies about pushed tags' do + ref = "#{Gitlab::Git::TAG_REF_PREFIX}test" + push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, []) + + chat_service.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when disabled' do + before do + chat_service.notify_only_default_branch = false + end + + it 'notifies about all push events' do + ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test" + push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, []) + + chat_service.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + end + end + describe "Note events" do let(:user) { create(:user) } let(:project) { create(:project, :repository, creator: user) } @@ -394,23 +458,6 @@ RSpec.shared_examples 'slack or mattermost notifications' do expect(result).to be_falsy end - - it 'does not notify push events if they are not for the default branch' do - ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test" - push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, []) - - chat_service.execute(push_sample_data) - - expect(WebMock).not_to have_requested(:post, webhook_url) - end - - it 'notifies about push events for the default branch' do - push_sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) - - chat_service.execute(push_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end end context 'when disabled' do |