diff options
author | Constance Okoghenun <cokoghenun@gitlab.com> | 2018-02-26 19:06:42 +0100 |
---|---|---|
committer | Constance Okoghenun <cokoghenun@gitlab.com> | 2018-02-26 19:06:42 +0100 |
commit | 8c122d65bfed5c5addd3245304895b98f8b83b36 (patch) | |
tree | 430ac6f1b2c30d0e0b8f0c6efa494918850e6004 | |
parent | fd598fd6bda91343d7e271c0fe485ead9e0011d2 (diff) | |
parent | 1553a34dbff167978f5dc81cc3a21e0b3b2b2bfa (diff) | |
download | gitlab-ce-8c122d65bfed5c5addd3245304895b98f8b83b36.tar.gz |
Resolved conflicts in app/assets/javascripts/pages/projects/settings/repository/show/index.js
98 files changed, 1133 insertions, 651 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 869884f8ca6..c8d399b2b98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,27 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 10.5.2 (2018-02-25) + +### Fixed (7 changes) + +- Fix single digit value clipping for stacked progress bar. !17217 +- Fix issue with cache key being empty when variable used as the key. !17260 +- Enable Legacy Authorization by default on Cluster creations. !17302 +- Allow branch names to be named the same as the sha it points to. +- Fix 500 error when loading an invalid upload URL. +- Don't attempt to update user tracked fields if database is in read-only. +- Prevent MR Widget error when no CI configured. + +### Performance (5 changes) + +- Improve query performance for snippets dashboard. !17088 +- Only check LFS integrity for first ref in a push to avoid timeout. !17098 +- Improve query performance of MembersFinder. !17190 +- Increase feature flag cache TTL to one hour. +- Improve performance of searching for and autocompleting of users. + + ## 10.5.1 (2018-02-22) - No changes. diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 92fc430ae8f..137c1281121 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.82.0 +0.85.0 @@ -411,7 +411,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.84.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.85.0', require: 'gitaly' # Locked until https://github.com/google/protobuf/issues/4210 is closed gem 'google-protobuf', '= 3.5.1' diff --git a/Gemfile.lock b/Gemfile.lock index 57ff086f0b1..89b86ae0259 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -285,7 +285,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly-proto (0.84.0) + gitaly-proto (0.85.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (5.3.3) @@ -1057,7 +1057,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly-proto (~> 0.84.0) + gitaly-proto (~> 0.85.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.6.2) diff --git a/app/assets/javascripts/boards/boards_bundle.js b/app/assets/javascripts/boards/index.js index 90166b3d3d1..06a8abea940 100644 --- a/app/assets/javascripts/boards/boards_bundle.js +++ b/app/assets/javascripts/boards/index.js @@ -24,7 +24,7 @@ import './components/new_list_dropdown'; import './components/modal/index'; import '../vue_shared/vue_resource_interceptor'; -$(() => { +export default () => { const $boardApp = document.getElementById('board-app'); const Store = gl.issueBoards.BoardsStore; const ModalStore = gl.issueBoards.ModalStore; @@ -236,4 +236,4 @@ $(() => { </div> `, }); -}); +}; diff --git a/app/assets/javascripts/deploy_keys/index.js b/app/assets/javascripts/deploy_keys/index.js index ca8798facc9..b727261648c 100644 --- a/app/assets/javascripts/deploy_keys/index.js +++ b/app/assets/javascripts/deploy_keys/index.js @@ -1,7 +1,7 @@ import Vue from 'vue'; import deployKeysApp from './components/app.vue'; -document.addEventListener('DOMContentLoaded', () => new Vue({ +export default () => new Vue({ el: document.getElementById('js-deploy-keys'), components: { deployKeysApp, @@ -18,4 +18,4 @@ document.addEventListener('DOMContentLoaded', () => new Vue({ }, }); }, -})); +}); diff --git a/app/assets/javascripts/filtered_search/components/recent_searches_dropdown_content.js b/app/assets/javascripts/filtered_search/components/recent_searches_dropdown_content.js deleted file mode 100644 index b693084e434..00000000000 --- a/app/assets/javascripts/filtered_search/components/recent_searches_dropdown_content.js +++ /dev/null @@ -1,102 +0,0 @@ -import eventHub from '../event_hub'; -import FilteredSearchTokenizer from '../filtered_search_tokenizer'; - -export default { - name: 'RecentSearchesDropdownContent', - - props: { - items: { - type: Array, - required: true, - }, - isLocalStorageAvailable: { - type: Boolean, - required: false, - default: true, - }, - allowedKeys: { - type: Array, - required: true, - }, - }, - - computed: { - processedItems() { - return this.items.map((item) => { - const { tokens, searchToken } - = FilteredSearchTokenizer.processTokens(item, this.allowedKeys); - - const resultantTokens = tokens.map(token => ({ - prefix: `${token.key}:`, - suffix: `${token.symbol}${token.value}`, - })); - - return { - text: item, - tokens: resultantTokens, - searchToken, - }; - }); - }, - hasItems() { - return this.items.length > 0; - }, - }, - - methods: { - onItemActivated(text) { - eventHub.$emit('recentSearchesItemSelected', text); - }, - onRequestClearRecentSearches(e) { - // Stop the dropdown from closing - e.stopPropagation(); - - eventHub.$emit('requestClearRecentSearches'); - }, - }, - - template: ` - <div> - <div - v-if="!isLocalStorageAvailable" - class="dropdown-info-note"> - This feature requires local storage to be enabled - </div> - <ul v-else-if="hasItems"> - <li - v-for="(item, index) in processedItems" - :key="index"> - <button - type="button" - class="filtered-search-history-dropdown-item" - @click="onItemActivated(item.text)"> - <span> - <span - v-for="(token, tokenIndex) in item.tokens" - class="filtered-search-history-dropdown-token"> - <span class="name">{{ token.prefix }}</span><span class="value">{{ token.suffix }}</span> - </span> - </span> - <span class="filtered-search-history-dropdown-search-token"> - {{ item.searchToken }} - </span> - </button> - </li> - <li class="divider"></li> - <li> - <button - type="button" - class="filtered-search-history-clear-button" - @click="onRequestClearRecentSearches($event)"> - Clear recent searches - </button> - </li> - </ul> - <div - v-else - class="dropdown-info-note"> - You don't have any recent searches - </div> - </div> - `, -}; diff --git a/app/assets/javascripts/filtered_search/components/recent_searches_dropdown_content.vue b/app/assets/javascripts/filtered_search/components/recent_searches_dropdown_content.vue new file mode 100644 index 00000000000..26618af9515 --- /dev/null +++ b/app/assets/javascripts/filtered_search/components/recent_searches_dropdown_content.vue @@ -0,0 +1,104 @@ +<script> +import eventHub from '../event_hub'; +import FilteredSearchTokenizer from '../filtered_search_tokenizer'; + +export default { + name: 'RecentSearchesDropdownContent', + props: { + items: { + type: Array, + required: true, + }, + isLocalStorageAvailable: { + type: Boolean, + required: false, + default: true, + }, + allowedKeys: { + type: Array, + required: true, + }, + }, + computed: { + processedItems() { + return this.items.map((item) => { + const { tokens, searchToken } + = FilteredSearchTokenizer.processTokens(item, this.allowedKeys); + + const resultantTokens = tokens.map(token => ({ + prefix: `${token.key}:`, + suffix: `${token.symbol}${token.value}`, + })); + + return { + text: item, + tokens: resultantTokens, + searchToken, + }; + }); + }, + hasItems() { + return this.items.length > 0; + }, + }, + methods: { + onItemActivated(text) { + eventHub.$emit('recentSearchesItemSelected', text); + }, + onRequestClearRecentSearches(e) { + // Stop the dropdown from closing + e.stopPropagation(); + + eventHub.$emit('requestClearRecentSearches'); + }, + }, +}; +</script> +<template> + <div> + <div + v-if="!isLocalStorageAvailable" + class="dropdown-info-note"> + This feature requires local storage to be enabled + </div> + <ul v-else-if="hasItems"> + <li + v-for="(item, index) in processedItems" + :key="`processed-items-${index}`" + > + <button + type="button" + class="filtered-search-history-dropdown-item" + @click="onItemActivated(item.text)"> + <span> + <span + class="filtered-search-history-dropdown-token" + v-for="(token, index) in item.tokens" + :key="`dropdown-token-${index}`" + > + <span class="name">{{ token.prefix }}</span> + <span class="value">{{ token.suffix }}</span> + </span> + </span> + <span class="filtered-search-history-dropdown-search-token"> + {{ item.searchToken }} + </span> + </button> + </li> + <li class="divider"></li> + <li> + <button + type="button" + class="filtered-search-history-clear-button" + @click="onRequestClearRecentSearches($event)"> + Clear recent searches + </button> + </li> + </ul> + <div + v-else + class="dropdown-info-note"> + You don't have any recent searches + </div> + </div> +</template> diff --git a/app/assets/javascripts/filtered_search/recent_searches_root.js b/app/assets/javascripts/filtered_search/recent_searches_root.js index c99ed63c4af..f9338b82acf 100644 --- a/app/assets/javascripts/filtered_search/recent_searches_root.js +++ b/app/assets/javascripts/filtered_search/recent_searches_root.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import RecentSearchesDropdownContent from './components/recent_searches_dropdown_content'; +import RecentSearchesDropdownContent from './components/recent_searches_dropdown_content.vue'; import eventHub from './event_hub'; class RecentSearchesRoot { @@ -33,7 +33,7 @@ class RecentSearchesRoot { this.vm = new Vue({ el: this.wrapperElement, components: { - 'recent-searches-dropdown-content': RecentSearchesDropdownContent, + RecentSearchesDropdownContent, }, data() { return state; }, template: ` diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 5de48aa49a9..7151ac05a09 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -213,7 +213,7 @@ export default class LabelsSelect { } } if (label.duplicate) { - color = gl.DropdownUtils.duplicateLabelColor(label.color); + color = DropdownUtils.duplicateLabelColor(label.color); } else { if (label.color != null) { diff --git a/app/assets/javascripts/pages/projects/boards/index.js b/app/assets/javascripts/pages/projects/boards/index.js index 3aeeedbb45d..5cfe8723204 100644 --- a/app/assets/javascripts/pages/projects/boards/index.js +++ b/app/assets/javascripts/pages/projects/boards/index.js @@ -1,7 +1,9 @@ import UsersSelect from '~/users_select'; import ShortcutsNavigation from '~/shortcuts_navigation'; +import initBoards from '~/boards'; document.addEventListener('DOMContentLoaded', () => { new UsersSelect(); // eslint-disable-line no-new new ShortcutsNavigation(); // eslint-disable-line no-new + initBoards(); }); diff --git a/app/assets/javascripts/pages/projects/settings/repository/show/index.js b/app/assets/javascripts/pages/projects/settings/repository/show/index.js index f46372faed8..001128ead59 100644 --- a/app/assets/javascripts/pages/projects/settings/repository/show/index.js +++ b/app/assets/javascripts/pages/projects/settings/repository/show/index.js @@ -3,9 +3,11 @@ import ProtectedTagCreate from '~/protected_tags/protected_tag_create'; import ProtectedTagEditList from '~/protected_tags/protected_tag_edit_list'; import initSettingsPanels from '~/settings_panels'; +import initDeployKeys from '~/deploy_keys'; document.addEventListener('DOMContentLoaded', () => { new ProtectedTagCreate(); new ProtectedTagEditList(); + initDeployKeys(); initSettingsPanels(); }); diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index 689befc742e..14545824e74 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -9,13 +9,12 @@ export default class ShortcutsIssuable extends Shortcuts { super(); this.$replyField = isMergeRequest ? $('.js-main-target-form #note_note') : $('.js-main-target-form .js-vue-comment-form'); - this.editBtn = document.querySelector('.js-issuable-edit'); Mousetrap.bind('a', () => ShortcutsIssuable.openSidebarDropdown('assignee')); Mousetrap.bind('m', () => ShortcutsIssuable.openSidebarDropdown('milestone')); Mousetrap.bind('l', () => ShortcutsIssuable.openSidebarDropdown('labels')); Mousetrap.bind('r', this.replyWithSelectedText.bind(this)); - Mousetrap.bind('e', this.editIssue.bind(this)); + Mousetrap.bind('e', ShortcutsIssuable.editIssue); if (isMergeRequest) { this.enabledHelp.push('.hidden-shortcut.merge_requests'); @@ -58,10 +57,10 @@ export default class ShortcutsIssuable extends Shortcuts { return false; } - editIssue() { + static editIssue() { // Need to click the element as on issues, editing is inline // on merge request, editing is on a different page - this.editBtn.click(); + document.querySelector('.js-issuable-edit').click(); return false; } diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js index 7ba6c29006a..162f048aac7 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js @@ -227,7 +227,8 @@ export default { @click="handleMergeButtonClick()" :disabled="isMergeButtonDisabled" :class="mergeButtonClass" - type="button"> + type="button" + class="qa-merge-button"> <i v-if="isMakingRequest" class="fa fa-spinner fa-spin" diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue index e9f23b0b113..143fd328d88 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue @@ -111,7 +111,7 @@ js-toggle-container accept-action media space-children" > <button type="button" - class="btn btn-sm btn-reopen btn-success" + class="btn btn-sm btn-reopen btn-success qa-mr-rebase-button" :disabled="isMakingRequest" @click="rebase" > diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 6763af4e98b..b9390450477 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -13,10 +13,20 @@ display: inline-block; } + .issuable-meta { + .author_link { + display: inline-block; + } + + .issuable-comments { + height: 18px; + } + } + .icon-merge-request-unmerged { height: 13px; margin-bottom: 3px; - } + } } } diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index f013e177c5b..5c9fce211ec 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -39,7 +39,7 @@ class LabelsFinder < UnionFinder end end elsif only_group_labels? - label_ids << Label.where(group_id: group.id) + label_ids << Label.where(group_id: group_ids) else label_ids << Label.where(group_id: projects.group_ids) label_ids << Label.where(project_id: projects.select(:id)) @@ -59,10 +59,11 @@ class LabelsFinder < UnionFinder items.where(title: title) end - def group - strong_memoize(:group) do + def group_ids + strong_memoize(:group_ids) do group = Group.find(params[:group_id]) - authorized_to_read_labels?(group) && group + groups = params[:include_ancestor_groups].present? ? group.self_and_ancestors : [group] + groups_user_can_read_labels(groups).map(&:id) end end @@ -120,4 +121,10 @@ class LabelsFinder < UnionFinder Ability.allowed?(current_user, :read_label, label_parent) end + + def groups_user_can_read_labels(groups) + DeclarativePolicy.user_scope do + groups.select { |group| authorized_to_read_labels?(group) } + end + end end diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index a6e1de6ffdc..0e806d16bc5 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -12,75 +12,42 @@ module BlobHelper def edit_blob_path(project = @project, ref = @ref, path = @path, options = {}) project_edit_blob_path(project, - tree_join(ref, path), - options[:link_opts]) - end - - def edit_blob_link(project = @project, ref = @ref, path = @path, options = {}) - blob = options.delete(:blob) - blob ||= project.repository.blob_at(ref, path) rescue nil - - return unless blob && blob.readable_text? - - common_classes = "btn js-edit-blob #{options[:extra_class]}" - - if !on_top_of_branch?(project, ref) - button_tag 'Edit', class: "#{common_classes} disabled has-tooltip", title: "You can only edit files when you are on a branch", data: { container: 'body' } - # This condition applies to anonymous or users who can edit directly - elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) - link_to 'Edit', edit_blob_path(project, ref, path, options), class: "#{common_classes} btn-sm" - elsif current_user && can?(current_user, :fork_project, project) - continue_params = { - to: edit_blob_path(project, ref, path, options), - notice: edit_in_new_fork_notice, - notice_now: edit_in_new_fork_notice_now - } - fork_path = project_forks_path(project, namespace_key: current_user.namespace.id, continue: continue_params) - - button_tag 'Edit', - class: "#{common_classes} js-edit-blob-link-fork-toggler", - data: { action: 'edit', fork_path: fork_path } - end + tree_join(ref, path), + options[:link_opts]) end def ide_edit_path(project = @project, ref = @ref, path = @path, options = {}) "#{ide_path}/project#{edit_blob_path(project, ref, path, options)}" end - def ide_edit_text - "#{_('Web IDE')}" - end + def edit_blob_button(project = @project, ref = @ref, path = @path, options = {}) + return unless blob = readable_blob(options, path, project, ref) - def ide_blob_link(project = @project, ref = @ref, path = @path, options = {}) - return unless show_new_ide? + common_classes = "btn js-edit-blob #{options[:extra_class]}" - blob = options.delete(:blob) - blob ||= project.repository.blob_at(ref, path) rescue nil + edit_button_tag(blob, + common_classes, + _('Edit'), + edit_blob_path(project, ref, path, options), + project, + ref) + end - return unless blob && blob.readable_text? + def ide_edit_button(project = @project, ref = @ref, path = @path, options = {}) + return unless show_new_ide? + return unless blob = readable_blob(options, path, project, ref) common_classes = "btn js-edit-ide #{options[:extra_class]}" - if !on_top_of_branch?(project, ref) - button_tag ide_edit_text, class: "#{common_classes} disabled has-tooltip", title: _('You can only edit files when you are on a branch'), data: { container: 'body' } - # This condition applies to anonymous or users who can edit directly - elsif current_user && can_modify_blob?(blob, project, ref) - link_to ide_edit_text, ide_edit_path(project, ref, path, options), class: "#{common_classes} btn-sm" - elsif current_user && can?(current_user, :fork_project, project) - continue_params = { - to: ide_edit_path(project, ref, path, options), - notice: edit_in_new_fork_notice, - notice_now: edit_in_new_fork_notice_now - } - fork_path = project_forks_path(project, namespace_key: current_user.namespace.id, continue: continue_params) - - button_tag ide_edit_text, - class: common_classes, - data: { fork_path: fork_path } - end + edit_button_tag(blob, + common_classes, + _('Web IDE'), + ide_edit_path(project, ref, path, options), + project, + ref) end - def modify_file_link(project = @project, ref = @ref, path = @path, label:, action:, btn_class:, modal_type:) + def modify_file_button(project = @project, ref = @ref, path = @path, label:, action:, btn_class:, modal_type:) return unless current_user blob = project.repository.blob_at(ref, path) rescue nil @@ -96,21 +63,12 @@ module BlobHelper elsif can_modify_blob?(blob, project, ref) button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal' elsif can?(current_user, :fork_project, project) - continue_params = { - to: request.fullpath, - notice: edit_in_new_fork_notice + " Try to #{action} this file again.", - notice_now: edit_in_new_fork_notice_now - } - fork_path = project_forks_path(project, namespace_key: current_user.namespace.id, continue: continue_params) - - button_tag label, - class: "#{common_classes} js-edit-blob-link-fork-toggler", - data: { action: action, fork_path: fork_path } + edit_fork_button_tag(common_classes, project, label, edit_modify_file_fork_params(action), action) end end def replace_blob_link(project = @project, ref = @ref, path = @path) - modify_file_link( + modify_file_button( project, ref, path, @@ -122,7 +80,7 @@ module BlobHelper end def delete_blob_link(project = @project, ref = @ref, path = @path) - modify_file_link( + modify_file_button( project, ref, path, @@ -332,4 +290,55 @@ module BlobHelper options end + + def readable_blob(options, path, project, ref) + blob = options.delete(:blob) + blob ||= project.repository.blob_at(ref, path) rescue nil + + blob if blob&.readable_text? + end + + def edit_blob_fork_params(path) + { + to: path, + notice: edit_in_new_fork_notice, + notice_now: edit_in_new_fork_notice_now + } + end + + def edit_modify_file_fork_params(action) + { + to: request.fullpath, + notice: edit_in_new_fork_notice_action(action), + notice_now: edit_in_new_fork_notice_now + } + end + + def edit_fork_button_tag(common_classes, project, label, params, action = 'edit') + fork_path = project_forks_path(project, namespace_key: current_user.namespace.id, continue: params) + + button_tag label, + class: "#{common_classes} js-edit-blob-link-fork-toggler", + data: { action: action, fork_path: fork_path } + end + + def edit_disabled_button_tag(button_text, common_classes) + button_tag(button_text, class: "#{common_classes} disabled has-tooltip", title: _('You can only edit files when you are on a branch'), data: { container: 'body' }) + end + + def edit_link_tag(link_text, edit_path, common_classes) + link_to link_text, edit_path, class: "#{common_classes} btn-sm" + end + + def edit_button_tag(blob, common_classes, text, edit_path, project, ref) + if !on_top_of_branch?(project, ref) + edit_disabled_button_tag(text, common_classes) + # This condition only applies to users who are logged in + # Web IDE (Beta) requires the user to have this feature enabled + elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) + edit_link_tag(text, edit_path, common_classes) + elsif current_user && can?(current_user, :fork_project, project) + edit_fork_button_tag(common_classes, project, text, edit_blob_fork_params(edit_path)) + end + end end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 7cd84fe69c9..44ecc2212f2 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -234,7 +234,7 @@ module IssuablesHelper data.merge!(updated_at_by(issuable)) - data.to_json + data end def updated_at_by(issuable) diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index f5733b4b57c..f6a6d9bebde 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -83,6 +83,10 @@ module TreeHelper " A fork of this project has been created that you can make changes in, so you can submit a merge request." end + def edit_in_new_fork_notice_action(action) + edit_in_new_fork_notice + " Try to #{action} this file again." + end + def commit_in_fork_help "A new branch will be created in your fork and a new merge request will be started." end diff --git a/app/models/tree.rb b/app/models/tree.rb index c89b8eca9be..4c1856b67a8 100644 --- a/app/models/tree.rb +++ b/app/models/tree.rb @@ -9,10 +9,9 @@ class Tree @repository = repository @sha = sha @path = path - @recursive = recursive git_repo = @repository.raw_repository - @entries = get_entries(git_repo, @sha, @path, recursive: @recursive) + @entries = Gitlab::Git::Tree.where(git_repo, @sha, @path, recursive) end def readme @@ -58,21 +57,4 @@ class Tree def sorted_entries trees + blobs + submodules end - - private - - def get_entries(git_repo, sha, path, recursive: false) - current_path_entries = Gitlab::Git::Tree.where(git_repo, sha, path) - ordered_entries = [] - - current_path_entries.each do |entry| - ordered_entries << entry - - if recursive && entry.dir? - ordered_entries.concat(get_entries(git_repo, sha, entry.path, recursive: true)) - end - end - - ordered_entries - end end diff --git a/app/services/ci/create_trace_artifact_service.rb b/app/services/ci/create_trace_artifact_service.rb index 280a2c3afa4..ffde824972c 100644 --- a/app/services/ci/create_trace_artifact_service.rb +++ b/app/services/ci/create_trace_artifact_service.rb @@ -4,13 +4,33 @@ module Ci return if job.job_artifacts_trace job.trace.read do |stream| - if stream.file? - job.create_job_artifacts_trace!( - project: job.project, - file_type: :trace, - file: stream) + break unless stream.file? + + clone_file!(stream.path, JobArtifactUploader.workhorse_upload_path) do |clone_path| + create_job_trace!(job, clone_path) + FileUtils.rm(stream.path) end end end + + private + + def create_job_trace!(job, path) + File.open(path) do |stream| + job.create_job_artifacts_trace!( + project: job.project, + file_type: :trace, + file: stream) + end + end + + def clone_file!(src_path, temp_dir) + FileUtils.mkdir_p(temp_dir) + Dir.mktmpdir('tmp-trace', temp_dir) do |dir_path| + temp_path = File.join(dir_path, "job.log") + FileUtils.copy(src_path, temp_path) + yield(temp_path) + end + end end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 66a9b1f82e0..e87fd49d193 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -77,8 +77,12 @@ class IssuableBaseService < BaseService return unless labels params[:label_ids] = labels.split(",").map do |label_name| - service = Labels::FindOrCreateService.new(current_user, project, title: label_name.strip) - label = service.execute + label = Labels::FindOrCreateService.new( + current_user, + parent, + title: label_name.strip, + available_labels: available_labels + ).execute label.try(:id) end.compact @@ -102,7 +106,7 @@ class IssuableBaseService < BaseService end def available_labels - LabelsFinder.new(current_user, project_id: @project.id).execute + @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute end def merge_quick_actions_into_params!(issuable) @@ -303,4 +307,8 @@ class IssuableBaseService < BaseService def update_project_counter_caches?(issuable) issuable.state_changed? end + + def parent + project + end end diff --git a/app/services/labels/find_or_create_service.rb b/app/services/labels/find_or_create_service.rb index 940c8b333d3..079f611b3f3 100644 --- a/app/services/labels/find_or_create_service.rb +++ b/app/services/labels/find_or_create_service.rb @@ -1,8 +1,9 @@ module Labels class FindOrCreateService - def initialize(current_user, project, params = {}) + def initialize(current_user, parent, params = {}) @current_user = current_user - @project = project + @parent = parent + @available_labels = params.delete(:available_labels) @params = params.dup.with_indifferent_access end @@ -13,12 +14,13 @@ module Labels private - attr_reader :current_user, :project, :params, :skip_authorization + attr_reader :current_user, :parent, :params, :skip_authorization def available_labels @available_labels ||= LabelsFinder.new( current_user, - project_id: project.id + "#{parent_type}_id".to_sym => parent.id, + only_group_labels: parent_is_group? ).execute(skip_authorization: skip_authorization) end @@ -27,8 +29,8 @@ module Labels def find_or_create_label new_label = available_labels.find_by(title: title) - if new_label.nil? && (skip_authorization || Ability.allowed?(current_user, :admin_label, project)) - new_label = Labels::CreateService.new(params).execute(project: project) + if new_label.nil? && (skip_authorization || Ability.allowed?(current_user, :admin_label, parent)) + new_label = Labels::CreateService.new(params).execute(parent_type.to_sym => parent) end new_label @@ -37,5 +39,13 @@ module Labels def title params[:title] || params[:name] end + + def parent_type + parent.model_name.param_key + end + + def parent_is_group? + parent_type == "group" + end end end diff --git a/app/views/projects/_merge_request_fast_forward_settings.html.haml b/app/views/projects/_merge_request_fast_forward_settings.html.haml index 8129c72feb2..f455522d17c 100644 --- a/app/views/projects/_merge_request_fast_forward_settings.html.haml +++ b/app/views/projects/_merge_request_fast_forward_settings.html.haml @@ -3,7 +3,7 @@ .radio = label_tag :project_merge_method_ff do - = form.radio_button :merge_method, :ff, class: "js-merge-method-radio" + = form.radio_button :merge_method, :ff, class: "js-merge-method-radio qa-radio-button-merge-ff" %strong Fast-forward merge %br %span.descr diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml index 2a77dedd9a2..1b150ec3e5c 100644 --- a/app/views/projects/blob/_header.html.haml +++ b/app/views/projects/blob/_header.html.haml @@ -11,8 +11,8 @@ = view_on_environment_button(@commit.sha, @path, @environment) if @environment .btn-group{ role: "group" }< - = edit_blob_link - = ide_blob_link + = edit_blob_button + = ide_edit_button - if current_user = replace_blob_link = delete_blob_link diff --git a/app/views/projects/blob/_upload.html.haml b/app/views/projects/blob/_upload.html.haml index b3afd16f900..f1324c61500 100644 --- a/app/views/projects/blob/_upload.html.haml +++ b/app/views/projects/blob/_upload.html.haml @@ -27,6 +27,3 @@ - unless can?(current_user, :push_code, @project) .inline.prepend-left-10 = commit_in_fork_help - -- content_for :page_specific_javascripts do - = webpack_bundle_tag('blob') diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 0b01e38d23d..47bfcb21cf4 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -17,7 +17,7 @@ \ - if editable_diff?(diff_file) - link_opts = @merge_request.persisted? ? { from_merge_request_iid: @merge_request.iid } : {} - = edit_blob_link(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path, + = edit_blob_button(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path, blob: blob, link_opts: link_opts) - if image_diff && image_replaced diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 0931ceb1512..b947b91322d 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -85,7 +85,7 @@ .settings-content = form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { multipart: true, class: "merge-request-settings-form" }, authenticity_token: true do |f| = render 'merge_request_settings', form: f - = f.submit 'Save changes', class: "btn btn-save" + = f.submit 'Save changes', class: "btn btn-save qa-save-merge-request-changes" = render 'export', project: @project diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 7bc5c46d64a..d63443c9da5 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -55,7 +55,7 @@ .issue-details.issuable-details .detail-page-description.content-block - %script#js-issuable-app-initial-data{ type: "application/json" }= issuable_initial_data(@issue) + %script#js-issuable-app-initial-data{ type: "application/json" }= issuable_initial_data(@issue).to_json #js-issuable-app %h2.title= markdown_field(@issue, :title) - if @issue.description.present? diff --git a/app/views/projects/settings/repository/show.html.haml b/app/views/projects/settings/repository/show.html.haml index 3077203c2a6..235d532bf98 100644 --- a/app/views/projects/settings/repository/show.html.haml +++ b/app/views/projects/settings/repository/show.html.haml @@ -4,7 +4,6 @@ - content_for :page_specific_javascripts do = webpack_bundle_tag('common_vue') - = webpack_bundle_tag('deploy_keys') -# Protected branches & tags use a lot of nested partials. -# The shared parts of the views can be found in the `shared` directory. diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml index 05539dfed7c..39511435508 100644 --- a/app/views/projects/tree/_tree_header.html.haml +++ b/app/views/projects/tree/_tree_header.html.haml @@ -75,7 +75,7 @@ - if show_new_ide? = succeed " " do = link_to ide_edit_path(@project, @id), class: 'btn btn-default' do - = ide_edit_text + = _('Web IDE') = link_to s_('Commits|History'), project_commits_path(@project, @id), class: 'btn' diff --git a/app/views/shared/boards/_show.html.haml b/app/views/shared/boards/_show.html.haml index a10fc42b82d..3312254f5fb 100644 --- a/app/views/shared/boards/_show.html.haml +++ b/app/views/shared/boards/_show.html.haml @@ -6,7 +6,6 @@ - content_for :page_specific_javascripts do = webpack_bundle_tag 'common_vue' - = webpack_bundle_tag 'boards' %script#js-board-template{ type: "text/x-template" }= render "shared/boards/components/board" %script#js-board-modal-filter{ type: "text/x-template" }= render "shared/issuable/search_bar", type: :boards_modal diff --git a/app/workers/authorized_projects_worker.rb b/app/workers/authorized_projects_worker.rb index 09559e3b696..d7e24491516 100644 --- a/app/workers/authorized_projects_worker.rb +++ b/app/workers/authorized_projects_worker.rb @@ -1,42 +1,10 @@ class AuthorizedProjectsWorker include ApplicationWorker + prepend WaitableWorker - # Schedules multiple jobs and waits for them to be completed. - def self.bulk_perform_and_wait(args_list) - # Short-circuit: it's more efficient to do small numbers of jobs inline - return bulk_perform_inline(args_list) if args_list.size <= 3 - - waiter = Gitlab::JobWaiter.new(args_list.size) - - # Point all the bulk jobs at the same JobWaiter. Converts, [[1], [2], [3]] - # into [[1, "key"], [2, "key"], [3, "key"]] - waiting_args_list = args_list.map { |args| [*args, waiter.key] } - bulk_perform_async(waiting_args_list) - - waiter.wait - end - - # Performs multiple jobs directly. Failed jobs will be put into sidekiq so - # they can benefit from retries - def self.bulk_perform_inline(args_list) - failed = [] - - args_list.each do |args| - begin - new.perform(*args) - rescue - failed << args - end - end - - bulk_perform_async(failed) if failed.present? - end - - def perform(user_id, notify_key = nil) + def perform(user_id) user = User.find_by(id: user_id) user&.refresh_authorized_projects - ensure - Gitlab::JobWaiter.notify(notify_key, jid) if notify_key end end diff --git a/app/workers/concerns/waitable_worker.rb b/app/workers/concerns/waitable_worker.rb new file mode 100644 index 00000000000..48ebe862248 --- /dev/null +++ b/app/workers/concerns/waitable_worker.rb @@ -0,0 +1,44 @@ +module WaitableWorker + extend ActiveSupport::Concern + + module ClassMethods + # Schedules multiple jobs and waits for them to be completed. + def bulk_perform_and_wait(args_list, timeout: 10) + # Short-circuit: it's more efficient to do small numbers of jobs inline + return bulk_perform_inline(args_list) if args_list.size <= 3 + + waiter = Gitlab::JobWaiter.new(args_list.size) + + # Point all the bulk jobs at the same JobWaiter. Converts, [[1], [2], [3]] + # into [[1, "key"], [2, "key"], [3, "key"]] + waiting_args_list = args_list.map { |args| [*args, waiter.key] } + bulk_perform_async(waiting_args_list) + + waiter.wait(timeout) + end + + # Performs multiple jobs directly. Failed jobs will be put into sidekiq so + # they can benefit from retries + def bulk_perform_inline(args_list) + failed = [] + + args_list.each do |args| + begin + new.perform(*args) + rescue + failed << args + end + end + + bulk_perform_async(failed) if failed.present? + end + end + + def perform(*args) + notify_key = args.pop if Gitlab::JobWaiter.key?(args.last) + + super(*args) + ensure + Gitlab::JobWaiter.notify(notify_key, jid) if notify_key + end +end diff --git a/changelogs/unreleased/34130-null-pipes.yml b/changelogs/unreleased/34130-null-pipes.yml deleted file mode 100644 index a56e5cf8db2..00000000000 --- a/changelogs/unreleased/34130-null-pipes.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Prevent MR Widget error when no CI configured -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/41461-project-members-slow-due-to-sql.yml b/changelogs/unreleased/41461-project-members-slow-due-to-sql.yml deleted file mode 100644 index 27eee7d943b..00000000000 --- a/changelogs/unreleased/41461-project-members-slow-due-to-sql.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improve query performance of MembersFinder. -merge_request: 17190 -author: -type: performance diff --git a/changelogs/unreleased/41619-turn-on-legacy-authorization-for-new-clusters-on-gke.yml b/changelogs/unreleased/41619-turn-on-legacy-authorization-for-new-clusters-on-gke.yml deleted file mode 100644 index 507367c98c4..00000000000 --- a/changelogs/unreleased/41619-turn-on-legacy-authorization-for-new-clusters-on-gke.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Enable Legacy Authorization by default on Cluster creations -merge_request: 17302 -author: -type: fixed diff --git a/changelogs/unreleased/42877-snippets-dashboard-slow.yml b/changelogs/unreleased/42877-snippets-dashboard-slow.yml deleted file mode 100644 index 839b44ad272..00000000000 --- a/changelogs/unreleased/42877-snippets-dashboard-slow.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improve query performance for snippets dashboard. -merge_request: 17088 -author: -type: performance diff --git a/changelogs/unreleased/43373-fix-cache-index-appending.yml b/changelogs/unreleased/43373-fix-cache-index-appending.yml deleted file mode 100644 index fdb293ea04d..00000000000 --- a/changelogs/unreleased/43373-fix-cache-index-appending.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix issue with cache key being empty when variable used as the key -merge_request: 17260 -author: -type: fixed diff --git a/changelogs/unreleased/43598-fix-duplicate-label-load-failure.yml b/changelogs/unreleased/43598-fix-duplicate-label-load-failure.yml new file mode 100644 index 00000000000..bda4ec84e5c --- /dev/null +++ b/changelogs/unreleased/43598-fix-duplicate-label-load-failure.yml @@ -0,0 +1,5 @@ +--- +title: Fix Group labels load failure when there are duplicate labels present +merge_request: 17353 +author: +type: fixed diff --git a/changelogs/unreleased/fix-500-for-invalid-upload-path.yml b/changelogs/unreleased/fix-500-for-invalid-upload-path.yml deleted file mode 100644 index a4ce00c64c4..00000000000 --- a/changelogs/unreleased/fix-500-for-invalid-upload-path.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix 500 error when loading an invalid upload URL -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/flipper-caching.yml b/changelogs/unreleased/flipper-caching.yml deleted file mode 100644 index 6db27fd579e..00000000000 --- a/changelogs/unreleased/flipper-caching.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Increase feature flag cache TTL to one hour -merge_request: -author: -type: performance diff --git a/changelogs/unreleased/grpc-unavailable-restart.yml b/changelogs/unreleased/grpc-unavailable-restart.yml new file mode 100644 index 00000000000..5ce08d66004 --- /dev/null +++ b/changelogs/unreleased/grpc-unavailable-restart.yml @@ -0,0 +1,5 @@ +--- +title: Restart Unicorn and Sidekiq when GRPC throws 14:Endpoint read failed +merge_request: 17293 +author: +type: fixed diff --git a/changelogs/unreleased/issue-edit-shortcut.yml b/changelogs/unreleased/issue-edit-shortcut.yml new file mode 100644 index 00000000000..2b29b2bc03f --- /dev/null +++ b/changelogs/unreleased/issue-edit-shortcut.yml @@ -0,0 +1,5 @@ +--- +title: Fixed issue edit shortcut not opening edit form +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/jej-fix-slow-lfs-object-check.yml b/changelogs/unreleased/jej-fix-slow-lfs-object-check.yml deleted file mode 100644 index 09112fba85e..00000000000 --- a/changelogs/unreleased/jej-fix-slow-lfs-object-check.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Only check LFS integrity for first ref in a push to avoid timeout -merge_request: 17098 -author: -type: performance diff --git a/changelogs/unreleased/kp-fix-stacked-bar-progress-value-clipping.yml b/changelogs/unreleased/kp-fix-stacked-bar-progress-value-clipping.yml deleted file mode 100644 index 690536a533b..00000000000 --- a/changelogs/unreleased/kp-fix-stacked-bar-progress-value-clipping.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix single digit value clipping for stacked progress bar -merge_request: 17217 -author: -type: fixed diff --git a/changelogs/unreleased/minimal-fix-for-artifacts-service.yml b/changelogs/unreleased/minimal-fix-for-artifacts-service.yml new file mode 100644 index 00000000000..11f5bc17759 --- /dev/null +++ b/changelogs/unreleased/minimal-fix-for-artifacts-service.yml @@ -0,0 +1,5 @@ +--- +title: Prevent trace artifact migration to incur data loss +merge_request: 17313 +author: +type: fixed diff --git a/changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml b/changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml new file mode 100644 index 00000000000..a761d610da1 --- /dev/null +++ b/changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml @@ -0,0 +1,5 @@ +--- +title: Return a 404 instead of 403 if the repository does not exist on disk +merge_request: 17341 +author: +type: fixed diff --git a/changelogs/unreleased/refactor-move-filtered-search-vue-component.yml b/changelogs/unreleased/refactor-move-filtered-search-vue-component.yml new file mode 100644 index 00000000000..d65318d7ba1 --- /dev/null +++ b/changelogs/unreleased/refactor-move-filtered-search-vue-component.yml @@ -0,0 +1,5 @@ +--- +title: Move RecentSearchesDropdownContent vue component +merge_request: 16951 +author: George Tsiolis +type: performance diff --git a/changelogs/unreleased/sh-guard-read-only-user-updates.yml b/changelogs/unreleased/sh-guard-read-only-user-updates.yml deleted file mode 100644 index b8dbd840ed9..00000000000 --- a/changelogs/unreleased/sh-guard-read-only-user-updates.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Don't attempt to update user tracked fields if database is in read-only -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/users-autocomplete.yml b/changelogs/unreleased/users-autocomplete.yml deleted file mode 100644 index 2cb078a3a7c..00000000000 --- a/changelogs/unreleased/users-autocomplete.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improve performance of searching for and autocompleting of users -merge_request: -author: -type: performance diff --git a/changelogs/unreleased/zj-branch-contains-git-message.yml b/changelogs/unreleased/zj-branch-contains-git-message.yml deleted file mode 100644 index ce034e7ec87..00000000000 --- a/changelogs/unreleased/zj-branch-contains-git-message.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Allow branch names to be named the same as the sha it points to -merge_request: -author: -type: fixed diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 0f164e628f9..161fb185c9b 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -10,7 +10,7 @@ Sidekiq.configure_server do |config| config.server_middleware do |chain| chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS'] - chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] + chain.add Gitlab::SidekiqMiddleware::Shutdown chain.add Gitlab::SidekiqMiddleware::RequestStoreMiddleware unless ENV['SIDEKIQ_REQUEST_STORE'] == '0' chain.add Gitlab::SidekiqStatus::ServerMiddleware end diff --git a/config/webpack.config.js b/config/webpack.config.js index 7f3b279b062..6fee0b5128b 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -51,11 +51,10 @@ var config = { context: path.join(ROOT_PATH, 'app/assets/javascripts'), entry: { balsamiq_viewer: './blob/balsamiq_viewer.js', - blob: './blob_edit/blob_bundle.js', - boards: './boards/boards_bundle.js', + common: './commons/index.js', + common_vue: './vue_shared/vue_resource_interceptor.js', cycle_analytics: './cycle_analytics/cycle_analytics_bundle.js', commit_pipelines: './commit/pipelines/pipelines_bundle.js', - deploy_keys: './deploy_keys/index.js', diff_notes: './diff_notes/diff_notes_bundle.js', environments: './environments/environments_bundle.js', environments_folder: './environments/folder/environments_folder_bundle.js', diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index e3b10119090..d9a61aea6ef 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -2,7 +2,9 @@ [Gitaly](https://gitlab.com/gitlab-org/gitaly) (introduced in GitLab 9.0) is a service that provides high-level RPC access to Git -repositories. Gitaly is a mandatory component in GitLab 9.4 and newer. +repositories. Gitaly was optional when it was first introduced in +GitLab, but since GitLab 9.4 it is a mandatory component of the +application. GitLab components that access Git repositories (gitlab-rails, gitlab-shell, gitlab-workhorse) act as clients to Gitaly. End users do @@ -184,14 +186,20 @@ Gitaly logs on your Gitaly server (`sudo gitlab-ctl tail gitaly` or coming in. One sure way to trigger a Gitaly request is to clone a repository from your GitLab server over HTTP. -## Disabling or enabling the Gitaly service +## Disabling or enabling the Gitaly service in a cluster environment If you are running Gitaly [as a remote service](#running-gitaly-on-its-own-server) you may want to disable the local Gitaly service that runs on your Gitlab server by default. -To disable the Gitaly service in your Omnibus installation, add the -following line to `/etc/gitlab/gitlab.rb`: +> 'Disabling Gitaly' only makes sense when you run GitLab in a custom +cluster configuration, where different services run on different +machines. Disabling Gitaly on all machines in the cluster is not a +valid configuration. + +If you are setting up a GitLab cluster where Gitaly does not need to +run on all machines, you can disable the Gitaly service in your +Omnibus installation, add the following line to `/etc/gitlab/gitlab.rb`: ```ruby gitaly['enable'] = false @@ -200,11 +208,13 @@ gitaly['enable'] = false When you run `gitlab-ctl reconfigure` the Gitaly service will be disabled. -To disable the Gitaly service in an installation from source, add the -following to `/etc/default/gitlab`: +To disable the Gitaly service in a GitLab cluster where you installed +GitLab from source, add the following to `/etc/default/gitlab` on the +machine where you want to disable Gitaly. ```shell gitaly_enabled=false ``` -When you run `service gitlab restart` Gitaly will be disabled. +When you run `service gitlab restart` Gitaly will be disabled on this +particular machine. diff --git a/doc/api/projects.md b/doc/api/projects.md index 9e649efea9c..4f4ab906c1a 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1224,7 +1224,7 @@ POST /projects/:id/hooks | `note_events` | boolean | no | Trigger hook on note events | | `job_events` | boolean | no | Trigger hook on job events | | `pipeline_events` | boolean | no | Trigger hook on pipeline events | -| `wiki_events` | boolean | no | Trigger hook on wiki events | +| `wiki_page_events` | boolean | no | Trigger hook on wiki events | | `enable_ssl_verification` | boolean | no | Do SSL verification when triggering the hook | | `token` | string | no | Secret token to validate received payloads; this will not be returned in the response | diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index e3cbf017e55..ddb9cf433eb 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -2206,7 +2206,7 @@ module Gitlab with_worktree(squash_path, branch, sparse_checkout_files: diff_files, env: env) do # Apply diff of the `diff_range` to the worktree diff = run_git!(%W(diff --binary #{diff_range})) - run_git!(%w(apply --index), chdir: squash_path, env: env) do |stdin| + run_git!(%w(apply --index --whitespace=nowarn), chdir: squash_path, env: env) do |stdin| stdin.binmode stdin.write(diff) end diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb index ba6058fd3c9..b6ceb542dd1 100644 --- a/lib/gitlab/git/tree.rb +++ b/lib/gitlab/git/tree.rb @@ -14,14 +14,14 @@ module Gitlab # Uses rugged for raw objects # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/320 - def where(repository, sha, path = nil) + def where(repository, sha, path = nil, recursive = false) path = nil if path == '' || path == '/' Gitlab::GitalyClient.migrate(:tree_entries) do |is_enabled| if is_enabled - repository.gitaly_commit_client.tree_entries(repository, sha, path) + repository.gitaly_commit_client.tree_entries(repository, sha, path, recursive) else - tree_entries_from_rugged(repository, sha, path) + tree_entries_from_rugged(repository, sha, path, recursive) end end end @@ -57,7 +57,22 @@ module Gitlab end end - def tree_entries_from_rugged(repository, sha, path) + def tree_entries_from_rugged(repository, sha, path, recursive) + current_path_entries = get_tree_entries_from_rugged(repository, sha, path) + ordered_entries = [] + + current_path_entries.each do |entry| + ordered_entries << entry + + if recursive && entry.dir? + ordered_entries.concat(tree_entries_from_rugged(repository, sha, entry.path, true)) + end + end + + ordered_entries + end + + def get_tree_entries_from_rugged(repository, sha, path) commit = repository.lookup(sha) root_tree = commit.tree diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index bbdb593d4e2..6400089a22f 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -199,7 +199,7 @@ module Gitlab def check_repository_existence! unless repository.exists? - raise UnauthorizedError, ERROR_MESSAGES[:no_repo] + raise NotFoundError, ERROR_MESSAGES[:no_repo] end end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index c5d3e944f7d..9cd76630484 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -125,6 +125,8 @@ module Gitlab kwargs = yield(kwargs) if block_given? stub(service, storage).__send__(rpc, request, kwargs) # rubocop:disable GitlabSecurity/PublicSend + rescue GRPC::Unavailable => ex + handle_grpc_unavailable!(ex) ensure duration = Gitlab::Metrics::System.monotonic_time - start @@ -135,6 +137,27 @@ module Gitlab duration) end + def self.handle_grpc_unavailable!(ex) + status = ex.to_status + raise ex unless status.details == 'Endpoint read failed' + + # There is a bug in grpc 1.8.x that causes a client process to get stuck + # always raising '14:Endpoint read failed'. The only thing that we can + # do to recover is to restart the process. + # + # See https://gitlab.com/gitlab-org/gitaly/issues/1029 + + if Sidekiq.server? + raise Gitlab::SidekiqMiddleware::Shutdown::WantShutdown.new(ex.to_s) + else + # SIGQUIT requests a Unicorn worker to shut down gracefully after the current request. + Process.kill('QUIT', Process.pid) + end + + raise ex + end + private_class_method :handle_grpc_unavailable! + def self.current_transaction_labels Gitlab::Metrics::Transaction.current&.labels || {} end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 269a048cf5d..d60f57717b5 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -105,11 +105,12 @@ module Gitlab entry unless entry.oid.blank? end - def tree_entries(repository, revision, path) + def tree_entries(repository, revision, path, recursive) request = Gitaly::GetTreeEntriesRequest.new( repository: @gitaly_repo, revision: encode_binary(revision), - path: path.present? ? encode_binary(path) : '.' + path: path.present? ? encode_binary(path) : '.', + recursive: recursive ) response = GitalyClient.call(@repository.storage, :commit_service, :get_tree_entries, request, timeout: GitalyClient.medium_timeout) diff --git a/lib/gitlab/job_waiter.rb b/lib/gitlab/job_waiter.rb index f654508c391..f7a8eae0be4 100644 --- a/lib/gitlab/job_waiter.rb +++ b/lib/gitlab/job_waiter.rb @@ -15,16 +15,22 @@ module Gitlab # push to that array when done. Once the waiter has popped `count` items, it # knows all the jobs are done. class JobWaiter + KEY_PREFIX = "gitlab:job_waiter".freeze + def self.notify(key, jid) Gitlab::Redis::SharedState.with { |redis| redis.lpush(key, jid) } end + def self.key?(key) + key.is_a?(String) && key =~ /\A#{KEY_PREFIX}:\h{8}-\h{4}-\h{4}-\h{4}-\h{12}\z/ + end + attr_reader :key, :finished attr_accessor :jobs_remaining # jobs_remaining - the number of jobs left to wait for # key - The key of this waiter. - def initialize(jobs_remaining = 0, key = "gitlab:job_waiter:#{SecureRandom.uuid}") + def initialize(jobs_remaining = 0, key = "#{KEY_PREFIX}:#{SecureRandom.uuid}") @key = key @jobs_remaining = jobs_remaining @finished = [] diff --git a/lib/gitlab/sidekiq_middleware/memory_killer.rb b/lib/gitlab/sidekiq_middleware/memory_killer.rb deleted file mode 100644 index b89ae2505c9..00000000000 --- a/lib/gitlab/sidekiq_middleware/memory_killer.rb +++ /dev/null @@ -1,67 +0,0 @@ -module Gitlab - module SidekiqMiddleware - class MemoryKiller - # Default the RSS limit to 0, meaning the MemoryKiller is disabled - MAX_RSS = (ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] || 0).to_s.to_i - # Give Sidekiq 15 minutes of grace time after exceeding the RSS limit - GRACE_TIME = (ENV['SIDEKIQ_MEMORY_KILLER_GRACE_TIME'] || 15 * 60).to_s.to_i - # Wait 30 seconds for running jobs to finish during graceful shutdown - SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i - - # Create a mutex used to ensure there will be only one thread waiting to - # shut Sidekiq down - MUTEX = Mutex.new - - def call(worker, job, queue) - yield - - current_rss = get_rss - - return unless MAX_RSS > 0 && current_rss > MAX_RSS - - Thread.new do - # Return if another thread is already waiting to shut Sidekiq down - return unless MUTEX.try_lock - - Sidekiq.logger.warn "Sidekiq worker PID-#{pid} current RSS #{current_rss}"\ - " exceeds maximum RSS #{MAX_RSS} after finishing job #{worker.class} JID-#{job['jid']}" - Sidekiq.logger.warn "Sidekiq worker PID-#{pid} will stop fetching new jobs in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later" - - # Wait `GRACE_TIME` to give the memory intensive job time to finish. - # Then, tell Sidekiq to stop fetching new jobs. - wait_and_signal(GRACE_TIME, 'SIGSTP', 'stop fetching new jobs') - - # Wait `SHUTDOWN_WAIT` to give already fetched jobs time to finish. - # Then, tell Sidekiq to gracefully shut down by giving jobs a few more - # moments to finish, killing and requeuing them if they didn't, and - # then terminating itself. - wait_and_signal(SHUTDOWN_WAIT, 'SIGTERM', 'gracefully shut down') - - # Wait for Sidekiq to shutdown gracefully, and kill it if it didn't. - wait_and_signal(Sidekiq.options[:timeout] + 2, 'SIGKILL', 'die') - end - end - - private - - def get_rss - output, status = Gitlab::Popen.popen(%W(ps -o rss= -p #{pid}), Rails.root.to_s) - return 0 unless status.zero? - - output.to_i - end - - def wait_and_signal(time, signal, explanation) - Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" - sleep(time) - - Sidekiq.logger.warn "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" - Process.kill(signal, pid) - end - - def pid - Process.pid - end - end - end -end diff --git a/lib/gitlab/sidekiq_middleware/shutdown.rb b/lib/gitlab/sidekiq_middleware/shutdown.rb new file mode 100644 index 00000000000..c2b8d6de66e --- /dev/null +++ b/lib/gitlab/sidekiq_middleware/shutdown.rb @@ -0,0 +1,133 @@ +require 'mutex_m' + +module Gitlab + module SidekiqMiddleware + class Shutdown + extend Mutex_m + + # Default the RSS limit to 0, meaning the MemoryKiller is disabled + MAX_RSS = (ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] || 0).to_s.to_i + # Give Sidekiq 15 minutes of grace time after exceeding the RSS limit + GRACE_TIME = (ENV['SIDEKIQ_MEMORY_KILLER_GRACE_TIME'] || 15 * 60).to_s.to_i + # Wait 30 seconds for running jobs to finish during graceful shutdown + SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i + + # This exception can be used to request that the middleware start shutting down Sidekiq + WantShutdown = Class.new(StandardError) + + ShutdownWithoutRaise = Class.new(WantShutdown) + private_constant :ShutdownWithoutRaise + + # For testing only, to avoid race conditions (?) in Rspec mocks. + attr_reader :trace + + # We store the shutdown thread in a class variable to ensure that there + # can be only one shutdown thread in the process. + def self.create_shutdown_thread + mu_synchronize do + return unless @shutdown_thread.nil? + + @shutdown_thread = Thread.new { yield } + end + end + + # For testing only: so we can wait for the shutdown thread to finish. + def self.shutdown_thread + mu_synchronize { @shutdown_thread } + end + + # For testing only: so that we can reset the global state before each test. + def self.clear_shutdown_thread + mu_synchronize { @shutdown_thread = nil } + end + + def initialize + @trace = Queue.new if Rails.env.test? + end + + def call(worker, job, queue) + shutdown_exception = nil + + begin + yield + check_rss! + rescue WantShutdown => ex + shutdown_exception = ex + end + + return unless shutdown_exception + + self.class.create_shutdown_thread do + do_shutdown(worker, job, shutdown_exception) + end + + raise shutdown_exception unless shutdown_exception.is_a?(ShutdownWithoutRaise) + end + + private + + def do_shutdown(worker, job, shutdown_exception) + Sidekiq.logger.warn "Sidekiq worker PID-#{pid} shutting down because of #{shutdown_exception} after job "\ + "#{worker.class} JID-#{job['jid']}" + Sidekiq.logger.warn "Sidekiq worker PID-#{pid} will stop fetching new jobs in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later" + + # Wait `GRACE_TIME` to give the memory intensive job time to finish. + # Then, tell Sidekiq to stop fetching new jobs. + wait_and_signal(GRACE_TIME, 'SIGTSTP', 'stop fetching new jobs') + + # Wait `SHUTDOWN_WAIT` to give already fetched jobs time to finish. + # Then, tell Sidekiq to gracefully shut down by giving jobs a few more + # moments to finish, killing and requeuing them if they didn't, and + # then terminating itself. + wait_and_signal(SHUTDOWN_WAIT, 'SIGTERM', 'gracefully shut down') + + # Wait for Sidekiq to shutdown gracefully, and kill it if it didn't. + wait_and_signal(Sidekiq.options[:timeout] + 2, 'SIGKILL', 'die') + end + + def check_rss! + return unless MAX_RSS > 0 + + current_rss = get_rss + return unless current_rss > MAX_RSS + + raise ShutdownWithoutRaise.new("current RSS #{current_rss} exceeds maximum RSS #{MAX_RSS}") + end + + def get_rss + output, status = Gitlab::Popen.popen(%W(ps -o rss= -p #{pid}), Rails.root.to_s) + return 0 unless status.zero? + + output.to_i + end + + def wait_and_signal(time, signal, explanation) + Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" + sleep(time) + + Sidekiq.logger.warn "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" + kill(signal, pid) + end + + def pid + Process.pid + end + + def sleep(time) + if Rails.env.test? + @trace << [:sleep, time] + else + Kernel.sleep(time) + end + end + + def kill(signal, pid) + if Rails.env.test? + @trace << [:kill, signal, pid] + else + Process.kill(signal, pid) + end + end + end + end +end @@ -130,6 +130,7 @@ module QA autoload :DeployKeys, 'qa/page/project/settings/deploy_keys' autoload :SecretVariables, 'qa/page/project/settings/secret_variables' autoload :Runners, 'qa/page/project/settings/runners' + autoload :MergeRequest, 'qa/page/project/settings/merge_request' end module Issue @@ -145,6 +146,7 @@ module QA module MergeRequest autoload :New, 'qa/page/merge_request/new' + autoload :Show, 'qa/page/merge_request/show' end module Admin diff --git a/qa/qa/factory/base.rb b/qa/qa/factory/base.rb index bd66b74a164..afaa96b4541 100644 --- a/qa/qa/factory/base.rb +++ b/qa/qa/factory/base.rb @@ -22,7 +22,7 @@ module QA factory.fabricate!(*args) - return Factory::Product.populate!(self) + return Factory::Product.populate!(factory) end end diff --git a/qa/qa/factory/product.rb b/qa/qa/factory/product.rb index d004e642f9b..996b7f14f61 100644 --- a/qa/qa/factory/product.rb +++ b/qa/qa/factory/product.rb @@ -17,8 +17,9 @@ module QA def self.populate!(factory) new.tap do |product| - factory.attributes.each_value do |attribute| - product.instance_exec(&attribute.block).tap do |value| + factory.class.attributes.each_value do |attribute| + product.instance_exec(factory, attribute.block) do |factory, block| + value = block.call(factory) product.define_singleton_method(attribute.name) { value } end end diff --git a/qa/qa/factory/repository/push.rb b/qa/qa/factory/repository/push.rb index 2f4de4173d4..6e8905cde78 100644 --- a/qa/qa/factory/repository/push.rb +++ b/qa/qa/factory/repository/push.rb @@ -2,7 +2,7 @@ module QA module Factory module Repository class Push < Factory::Base - attr_writer :file_name, :file_content, :commit_message, :branch_name + attr_writer :file_name, :file_content, :commit_message, :branch_name, :new_branch dependency Factory::Resource::Project, as: :project do |project| project.name = 'project-with-code' @@ -14,6 +14,7 @@ module QA @file_content = '# This is test project' @commit_message = "Add #{@file_name}" @branch_name = 'master' + @new_branch = true end def fabricate! @@ -29,6 +30,7 @@ module QA repository.clone repository.configure_identity('GitLab QA', 'root@gitlab.com') + repository.checkout(@branch_name) unless @new_branch repository.add_file(@file_name, @file_content) repository.commit(@commit_message) repository.push_changes(@branch_name) diff --git a/qa/qa/factory/resource/merge_request.rb b/qa/qa/factory/resource/merge_request.rb index ce04e904aaf..539fe6b8a70 100644 --- a/qa/qa/factory/resource/merge_request.rb +++ b/qa/qa/factory/resource/merge_request.rb @@ -9,11 +9,20 @@ module QA :source_branch, :target_branch + product :project do |factory| + factory.project + end + + product :source_branch do |factory| + factory.source_branch + end + dependency Factory::Resource::Project, as: :project do |project| project.name = 'project-with-merge-request' end dependency Factory::Repository::Push, as: :target do |push, factory| + factory.project.visit! push.project = factory.project push.branch_name = "master:#{factory.target_branch}" end diff --git a/qa/qa/git/repository.rb b/qa/qa/git/repository.rb index 4c4ef3ef477..b3150e8f3fa 100644 --- a/qa/qa/git/repository.rb +++ b/qa/qa/git/repository.rb @@ -36,6 +36,10 @@ module QA `git clone #{opts} #{@uri.to_s} ./ #{suppress_output}` end + def checkout(branch_name) + `git checkout "#{branch_name}"` + end + def shallow_clone clone('--depth 1') end diff --git a/qa/qa/page/merge_request/show.rb b/qa/qa/page/merge_request/show.rb new file mode 100644 index 00000000000..35875487da8 --- /dev/null +++ b/qa/qa/page/merge_request/show.rb @@ -0,0 +1,46 @@ +module QA + module Page + module MergeRequest + class Show < Page::Base + view 'app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js' do + element :merge_button + end + + view 'app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue' do + element :merged_status, 'The changes were merged into' + end + + view 'app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue' do + element :mr_rebase_button + element :fast_forward_nessage, "Fast-forward merge is not possible" + end + + def rebase! + wait(reload: false) do + click_element :mr_rebase_button + + has_text?("The source branch HEAD has recently changed.") + end + end + + def fast_forward_possible? + !has_text?("Fast-forward merge is not possible") + end + + def has_merge_button? + refresh + + has_selector?('.accept-merge-request') + end + + def merge! + wait(reload: false) do + click_element :merge_button + + has_text?("The changes were merged into") + end + end + end + end + end +end diff --git a/qa/qa/page/project/settings/merge_request.rb b/qa/qa/page/project/settings/merge_request.rb new file mode 100644 index 00000000000..b147c91b467 --- /dev/null +++ b/qa/qa/page/project/settings/merge_request.rb @@ -0,0 +1,27 @@ +module QA + module Page + module Project + module Settings + class MergeRequest < QA::Page::Base + include Common + + view 'app/views/projects/_merge_request_fast_forward_settings.html.haml' do + element :radio_button_merge_ff + end + + view 'app/views/projects/edit.html.haml' do + element :merge_request_settings, 'Merge request settings' + element :save_merge_request_changes + end + + def enable_ff_only + expand_section('Merge request settings') do + click_element :radio_button_merge_ff + click_element :save_merge_request_changes + end + end + end + end + end + end +end diff --git a/qa/qa/specs/features/merge_request/rebase_spec.rb b/qa/qa/specs/features/merge_request/rebase_spec.rb new file mode 100644 index 00000000000..2a44d42af6f --- /dev/null +++ b/qa/qa/specs/features/merge_request/rebase_spec.rb @@ -0,0 +1,39 @@ +module QA + feature 'merge request rebase', :core do + scenario 'rebases source branch of merge request' do + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.act { sign_in_using_credentials } + + project = Factory::Resource::Project.fabricate! do |project| + project.name = "only-fast-forward" + end + + Page::Menu::Side.act { go_to_settings } + Page::Project::Settings::MergeRequest.act { enable_ff_only } + + merge_request = Factory::Resource::MergeRequest.fabricate! do |merge_request| + merge_request.project = project + merge_request.title = 'Needs rebasing' + end + + Factory::Repository::Push.fabricate! do |push| + push.project = project + push.file_name = "other.txt" + push.file_content = "New file added!" + end + + merge_request.visit! + + Page::MergeRequest::Show.perform do |merge_request| + expect(merge_request).to have_content('Needs rebasing') + expect(merge_request).not_to be_fast_forward_possible + expect(merge_request).not_to have_merge_button + + merge_request.rebase! + + expect(merge_request).to have_merge_button + expect(merge_request.fast_forward_possible?).to be_truthy + end + end + end +end diff --git a/qa/spec/factory/base_spec.rb b/qa/spec/factory/base_spec.rb index c5663049be8..04e04886699 100644 --- a/qa/spec/factory/base_spec.rb +++ b/qa/spec/factory/base_spec.rb @@ -7,6 +7,7 @@ describe QA::Factory::Base do before do allow(QA::Factory::Product).to receive(:new).and_return(product) + allow(QA::Factory::Product).to receive(:populate!).and_return(product) end it 'instantiates the factory and calls factory method' do @@ -76,6 +77,7 @@ describe QA::Factory::Base do allow(subject).to receive(:new).and_return(instance) allow(instance).to receive(:mydep).and_return(nil) allow(QA::Factory::Product).to receive(:new) + allow(QA::Factory::Product).to receive(:populate!) end it 'builds all dependencies first' do @@ -89,8 +91,16 @@ describe QA::Factory::Base do describe '.product' do subject do Class.new(described_class) do + def fabricate! + "any" + end + + # Defined only to be stubbed + def self.find_page + end + product :token do - page.do_something_on_page! + find_page.do_something_on_page! 'resulting value' end end @@ -105,16 +115,17 @@ describe QA::Factory::Base do let(:page) { spy('page') } before do - allow(subject).to receive(:new).and_return(factory) + allow(factory).to receive(:class).and_return(subject) allow(QA::Factory::Product).to receive(:new).and_return(product) allow(product).to receive(:page).and_return(page) + allow(subject).to receive(:find_page).and_return(page) end it 'populates product after fabrication' do subject.fabricate! - expect(page).to have_received(:do_something_on_page!) expect(product.token).to eq 'resulting value' + expect(page).to have_received(:do_something_on_page!) end end end diff --git a/qa/spec/factory/product_spec.rb b/qa/spec/factory/product_spec.rb index fdfb1ec90cc..f245aabbf43 100644 --- a/qa/spec/factory/product_spec.rb +++ b/qa/spec/factory/product_spec.rb @@ -1,9 +1,20 @@ describe QA::Factory::Product do - let(:factory) { spy('factory') } + let(:factory) do + QA::Factory::Base.new + end + + let(:attributes) do + { test: QA::Factory::Product::Attribute.new(:test, proc { 'returned' }) } + end + let(:product) { spy('product') } + before do + allow(QA::Factory::Base).to receive(:attributes).and_return(attributes) + end + describe '.populate!' do - it 'returns a fabrication product' do + it 'returns a fabrication product and define factory attributes as its methods' do expect(described_class).to receive(:new).and_return(product) result = described_class.populate!(factory) do |instance| @@ -11,6 +22,7 @@ describe QA::Factory::Product do end expect(result).to be product + expect(result.test).to eq('returned') end end diff --git a/spec/features/issues/filtered_search/recent_searches_spec.rb b/spec/features/issues/filtered_search/recent_searches_spec.rb index f355cec3ba9..41b9ada988a 100644 --- a/spec/features/issues/filtered_search/recent_searches_spec.rb +++ b/spec/features/issues/filtered_search/recent_searches_spec.rb @@ -39,8 +39,8 @@ describe 'Recent searches', :js do items = all('.filtered-search-history-dropdown-item', visible: false, count: 2) - expect(items[0].text).to eq('label:~qux garply') - expect(items[1].text).to eq('label:~foo bar') + expect(items[0].text).to eq('label: ~qux garply') + expect(items[1].text).to eq('label: ~foo bar') end it 'saved recent searches are restored last on the list' do diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index faf14be4818..c2c4b479a8a 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -271,6 +271,18 @@ describe 'New/edit issue', :js do end end + context 'inline edit' do + before do + visit project_issue_path(project, issue) + end + + it 'opens inline edit form with shortcut' do + find('body').send_keys('e') + + expect(page).to have_selector('.detail-page-description form') + end + end + describe 'sub-group project' do let(:group) { create(:group) } let(:nested_group_1) { create(:group, parent: group) } diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 645d12da09f..cfe979a8647 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -146,8 +146,8 @@ feature 'Project' do end describe 'removal', :js do - let(:user) { create(:user, username: 'test', name: 'test') } - let(:project) { create(:project, namespace: user.namespace, name: 'project1') } + let(:user) { create(:user) } + let(:project) { create(:project, namespace: user.namespace) } before do sign_in(user) @@ -156,8 +156,8 @@ feature 'Project' do end it 'removes a project' do - expect { remove_with_confirm('Remove project', project.path) }.to change {Project.count}.by(-1) - expect(page).to have_content "Project 'test / project1' is in the process of being deleted." + expect { remove_with_confirm('Remove project', project.path) }.to change { Project.count }.by(-1) + expect(page).to have_content "Project '#{project.full_name}' is in the process of being deleted." expect(Project.all.count).to be_zero expect(project.issues).to be_empty expect(project.merge_requests).to be_empty diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index 06031aee217..dc76efea35b 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/spec/finders/labels_finder_spec.rb @@ -5,6 +5,8 @@ describe LabelsFinder do let(:group_1) { create(:group) } let(:group_2) { create(:group) } let(:group_3) { create(:group) } + let(:private_group_1) { create(:group, :private) } + let(:private_subgroup_1) { create(:group, :private, parent: private_group_1) } let(:project_1) { create(:project, namespace: group_1) } let(:project_2) { create(:project, namespace: group_2) } @@ -20,6 +22,8 @@ describe LabelsFinder do let!(:group_label_1) { create(:group_label, group: group_1, title: 'Label 1 (group)') } let!(:group_label_2) { create(:group_label, group: group_1, title: 'Group Label 2') } let!(:group_label_3) { create(:group_label, group: group_2, title: 'Group Label 3') } + let!(:private_group_label_1) { create(:group_label, group: private_group_1, title: 'Private Group Label 1') } + let!(:private_subgroup_label_1) { create(:group_label, group: private_subgroup_1, title: 'Private Sub Group Label 1') } let(:user) { create(:user) } @@ -66,6 +70,25 @@ describe LabelsFinder do expect(finder.execute).to eq [group_label_2, group_label_1] end end + + context 'when including labels from group ancestors', :nested_groups do + it 'returns labels from group and its ancestors' do + private_group_1.add_developer(user) + private_subgroup_1.add_developer(user) + + finder = described_class.new(user, group_id: private_subgroup_1.id, only_group_labels: true, include_ancestor_groups: true) + + expect(finder.execute).to eq [private_group_label_1, private_subgroup_label_1] + end + + it 'ignores labels from groups which user can not read' do + private_subgroup_1.add_developer(user) + + finder = described_class.new(user, group_id: private_subgroup_1.id, only_group_labels: true, include_ancestor_groups: true) + + expect(finder.execute).to eq [private_subgroup_label_1] + end + end end context 'filtering by project_id' do diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index a030796c54e..1fa194fe1b8 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -86,7 +86,7 @@ describe BlobHelper do it 'verifies blob is text' do expect(helper).not_to receive(:blob_text_viewable?) - button = edit_blob_link(project, 'refs/heads/master', 'README.md') + button = edit_blob_button(project, 'refs/heads/master', 'README.md') expect(button).to start_with('<button') end @@ -96,17 +96,17 @@ describe BlobHelper do expect(project.repository).not_to receive(:blob_at) - edit_blob_link(project, 'refs/heads/master', 'README.md', blob: blob) + edit_blob_button(project, 'refs/heads/master', 'README.md', blob: blob) end it 'returns a link with the proper route' do - link = edit_blob_link(project, 'master', 'README.md') + link = edit_blob_button(project, 'master', 'README.md') expect(Capybara.string(link).find_link('Edit')[:href]).to eq("/#{project.full_path}/edit/master/README.md") end it 'returns a link with the passed link_opts on the expected route' do - link = edit_blob_link(project, 'master', 'README.md', link_opts: { mr_id: 10 }) + link = edit_blob_button(project, 'master', 'README.md', link_opts: { mr_id: 10 }) expect(Capybara.string(link).find_link('Edit')[:href]).to eq("/#{project.full_path}/edit/master/README.md?mr_id=10") end diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 7fa665aecdc..2fecd1a3d27 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -173,23 +173,23 @@ describe IssuablesHelper do @project = issue.project expected_data = { - 'endpoint' => "/#{@project.full_path}/issues/#{issue.iid}", - 'updateEndpoint' => "/#{@project.full_path}/issues/#{issue.iid}.json", - 'canUpdate' => true, - 'canDestroy' => true, - 'issuableRef' => "##{issue.iid}", - 'markdownPreviewPath' => "/#{@project.full_path}/preview_markdown", - 'markdownDocsPath' => '/help/user/markdown', - 'issuableTemplates' => [], - 'projectPath' => @project.path, - 'projectNamespace' => @project.namespace.path, - 'initialTitleHtml' => issue.title, - 'initialTitleText' => issue.title, - 'initialDescriptionHtml' => '<p dir="auto">issue text</p>', - 'initialDescriptionText' => 'issue text', - 'initialTaskStatus' => '0 of 0 tasks completed' + endpoint: "/#{@project.full_path}/issues/#{issue.iid}", + updateEndpoint: "/#{@project.full_path}/issues/#{issue.iid}.json", + canUpdate: true, + canDestroy: true, + issuableRef: "##{issue.iid}", + markdownPreviewPath: "/#{@project.full_path}/preview_markdown", + markdownDocsPath: '/help/user/markdown', + issuableTemplates: [], + projectPath: @project.path, + projectNamespace: @project.namespace.path, + initialTitleHtml: issue.title, + initialTitleText: issue.title, + initialDescriptionHtml: '<p dir="auto">issue text</p>', + initialDescriptionText: 'issue text', + initialTaskStatus: '0 of 0 tasks completed' } - expect(JSON.parse(helper.issuable_initial_data(issue))).to eq(expected_data) + expect(helper.issuable_initial_data(issue)).to eq(expected_data) end end diff --git a/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js b/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js index 4a516c517ef..59bd2650081 100644 --- a/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js +++ b/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js @@ -1,7 +1,6 @@ import Vue from 'vue'; import eventHub from '~/filtered_search/event_hub'; -import RecentSearchesDropdownContent from '~/filtered_search/components/recent_searches_dropdown_content'; - +import RecentSearchesDropdownContent from '~/filtered_search/components/recent_searches_dropdown_content.vue'; import FilteredSearchTokenKeys from '~/filtered_search/filtered_search_token_keys'; const createComponent = (propsData) => { diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 323b8a9572d..94fcc6c7f2b 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -113,7 +113,9 @@ if (process.env.BABEL_ENV === 'coverage') { // exempt these files from the coverage report const troubleMakers = [ './blob_edit/blob_bundle.js', - './boards/boards_bundle.js', + './boards/components/modal/empty_state.js', + './boards/components/modal/footer.js', + './boards/components/modal/header.js', './cycle_analytics/cycle_analytics_bundle.js', './cycle_analytics/components/stage_plan_component.js', './cycle_analytics/components/stage_staging_component.js', diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index d601a383a98..13358995383 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2283,6 +2283,20 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(subject).to match(/\h{40}/) end end + + context 'with trailing whitespace in an invalid patch', :skip_gitaly_mock do + let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+ \n ====== \n \n Sample repo for testing gitlab features\n" } + + it 'does not include whitespace warnings in the error' do + allow(repository).to receive(:run_git!).and_call_original + allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT')) + + expect { subject }.to raise_error do |error| + expect(error).to be_a(described_class::GitError) + expect(error.message).not_to include('trailing whitespace') + end + end + end end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 19d3f55501e..6f07e423c1b 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -534,6 +534,19 @@ describe Gitlab::GitAccess do expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.') end + context 'when the project repository does not exist' do + it 'returns not found' do + project.add_guest(user) + repo = project.repository + FileUtils.rm_rf(repo.path) + + # Sanity check for rm_rf + expect(repo.exists?).to eq(false) + + expect { pull_access_check }.to raise_error(Gitlab::GitAccess::NotFoundError, 'A repository for this project does not exist yet.') + end + end + describe 'without access to project' do context 'pull code' do it { expect { pull_access_check }.to raise_not_found } diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 215f1ecc9c5..730ede99fc9 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -57,7 +57,7 @@ describe Gitlab::GitAccessWiki do # Sanity check for rm_rf expect(wiki_repo.exists?).to eq(false) - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'A repository for this project does not exist yet.') + expect { subject }.to raise_error(Gitlab::GitAccess::NotFoundError, 'A repository for this project does not exist yet.') end end end diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 001c4d3e10a..9be3fa633a7 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -113,7 +113,7 @@ describe Gitlab::GitalyClient::CommitService do .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) .and_return([]) - client.tree_entries(repository, revision, path) + client.tree_entries(repository, revision, path, false) end context 'with UTF-8 params strings' do @@ -126,7 +126,7 @@ describe Gitlab::GitalyClient::CommitService do .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) .and_return([]) - client.tree_entries(repository, revision, path) + client.tree_entries(repository, revision, path, false) end end end diff --git a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb deleted file mode 100644 index 8fdbbacd04d..00000000000 --- a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb +++ /dev/null @@ -1,63 +0,0 @@ -require 'spec_helper' - -describe Gitlab::SidekiqMiddleware::MemoryKiller do - subject { described_class.new } - let(:pid) { 999 } - - let(:worker) { double(:worker, class: 'TestWorker') } - let(:job) { { 'jid' => 123 } } - let(:queue) { 'test_queue' } - - def run - thread = subject.call(worker, job, queue) { nil } - thread&.join - end - - before do - allow(subject).to receive(:get_rss).and_return(10.kilobytes) - allow(subject).to receive(:pid).and_return(pid) - end - - context 'when MAX_RSS is set to 0' do - before do - stub_const("#{described_class}::MAX_RSS", 0) - end - - it 'does nothing' do - expect(subject).not_to receive(:sleep) - - run - end - end - - context 'when MAX_RSS is exceeded' do - before do - stub_const("#{described_class}::MAX_RSS", 5.kilobytes) - end - - it 'sends the STP, TERM and KILL signals at expected times' do - expect(subject).to receive(:sleep).with(15 * 60).ordered - expect(Process).to receive(:kill).with('SIGSTP', pid).ordered - - expect(subject).to receive(:sleep).with(30).ordered - expect(Process).to receive(:kill).with('SIGTERM', pid).ordered - - expect(subject).to receive(:sleep).with(10).ordered - expect(Process).to receive(:kill).with('SIGKILL', pid).ordered - - run - end - end - - context 'when MAX_RSS is not exceeded' do - before do - stub_const("#{described_class}::MAX_RSS", 15.kilobytes) - end - - it 'does nothing' do - expect(subject).not_to receive(:sleep) - - run - end - end -end diff --git a/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb b/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb new file mode 100644 index 00000000000..0001795c3f0 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +describe Gitlab::SidekiqMiddleware::Shutdown do + subject { described_class.new } + + let(:pid) { Process.pid } + let(:worker) { double(:worker, class: 'TestWorker') } + let(:job) { { 'jid' => 123 } } + let(:queue) { 'test_queue' } + let(:block) { proc { nil } } + + def run + subject.call(worker, job, queue) { block.call } + described_class.shutdown_thread&.join + end + + def pop_trace + subject.trace.pop(true) + end + + before do + allow(subject).to receive(:get_rss).and_return(10.kilobytes) + described_class.clear_shutdown_thread + end + + context 'when MAX_RSS is set to 0' do + before do + stub_const("#{described_class}::MAX_RSS", 0) + end + + it 'does nothing' do + expect(subject).not_to receive(:sleep) + + run + end + end + + def expect_shutdown_sequence + expect(pop_trace).to eq([:sleep, 15 * 60]) + expect(pop_trace).to eq([:kill, 'SIGTSTP', pid]) + + expect(pop_trace).to eq([:sleep, 30]) + expect(pop_trace).to eq([:kill, 'SIGTERM', pid]) + + expect(pop_trace).to eq([:sleep, 10]) + expect(pop_trace).to eq([:kill, 'SIGKILL', pid]) + end + + context 'when MAX_RSS is exceeded' do + before do + stub_const("#{described_class}::MAX_RSS", 5.kilobytes) + end + + it 'sends the TSTP, TERM and KILL signals at expected times' do + run + + expect_shutdown_sequence + end + end + + context 'when MAX_RSS is not exceeded' do + before do + stub_const("#{described_class}::MAX_RSS", 15.kilobytes) + end + + it 'does nothing' do + expect(subject).not_to receive(:sleep) + + run + end + end + + context 'when WantShutdown is raised' do + let(:block) { proc { raise described_class::WantShutdown } } + + it 'starts the shutdown sequence and re-raises the exception' do + expect { run }.to raise_exception(described_class::WantShutdown) + + # We can't expect 'run' to have joined on the shutdown thread, because + # it hit an exception. + shutdown_thread = described_class.shutdown_thread + expect(shutdown_thread).not_to be_nil + shutdown_thread.join + + expect_shutdown_sequence + end + end +end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index c6fdda203ad..1b0a5eac9b0 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -597,7 +597,7 @@ describe 'Git HTTP requests' do context "when a gitlab ci token is provided" do let(:project) { create(:project, :repository) } let(:build) { create(:ci_build, :running) } - let(:other_project) { create(:project) } + let(:other_project) { create(:project, :repository) } before do build.update!(project: project) # can't associate it on factory create @@ -648,10 +648,10 @@ describe 'Git HTTP requests' do context 'when the repo does not exist' do let(:project) { create(:project) } - it 'rejects pulls with 403 Forbidden' do + it 'rejects pulls with 404 Not Found' do clone_get path, env - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) expect(response.body).to eq(git_access_error(:no_repo)) end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index de829011e58..6bed8e812c0 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -68,10 +68,10 @@ describe 'OpenID Connect requests' do let!(:public_email) { build :email, email: 'public@example.com' } let!(:private_email) { build :email, email: 'private@example.com' } - let!(:group1) { create :group, path: 'group1' } - let!(:group2) { create :group, path: 'group2' } - let!(:group3) { create :group, path: 'group3', parent: group2 } - let!(:group4) { create :group, path: 'group4', parent: group3 } + let!(:group1) { create :group } + let!(:group2) { create :group } + let!(:group3) { create :group, parent: group2 } + let!(:group4) { create :group, parent: group3 } before do group1.add_user(user, GroupMember::OWNER) @@ -93,8 +93,8 @@ describe 'OpenID Connect requests' do 'groups' => anything })) - expected_groups = %w[group1 group2/group3] - expected_groups << 'group2/group3/group4' if Group.supports_nested_groups? + expected_groups = [group1.full_path, group3.full_path] + expected_groups << group4.full_path if Group.supports_nested_groups? expect(json_response['groups']).to match_array(expected_groups) end end diff --git a/spec/services/ci/create_trace_artifact_service_spec.rb b/spec/services/ci/create_trace_artifact_service_spec.rb index 847a88920fe..8c5e8e438c7 100644 --- a/spec/services/ci/create_trace_artifact_service_spec.rb +++ b/spec/services/ci/create_trace_artifact_service_spec.rb @@ -4,40 +4,60 @@ describe Ci::CreateTraceArtifactService do describe '#execute' do subject { described_class.new(nil, nil).execute(job) } - let(:job) { create(:ci_build) } - context 'when the job does not have trace artifact' do context 'when the job has a trace file' do - before do - allow_any_instance_of(Gitlab::Ci::Trace) - .to receive(:default_path) { expand_fixture_path('trace/sample_trace') } + let!(:job) { create(:ci_build, :trace_live) } + let!(:legacy_path) { job.trace.read { |stream| return stream.path } } + let!(:legacy_checksum) { Digest::SHA256.file(legacy_path).hexdigest } + let(:new_path) { job.job_artifacts_trace.file.path } + let(:new_checksum) { Digest::SHA256.file(new_path).hexdigest } - allow_any_instance_of(JobArtifactUploader).to receive(:move_to_cache) { false } - allow_any_instance_of(JobArtifactUploader).to receive(:move_to_store) { false } - end + it { expect(File.exist?(legacy_path)).to be_truthy } it 'creates trace artifact' do expect { subject }.to change { Ci::JobArtifact.count }.by(1) - expect(job.job_artifacts_trace.read_attribute(:file)).to eq('sample_trace') + expect(File.exist?(legacy_path)).to be_falsy + expect(File.exist?(new_path)).to be_truthy + expect(new_checksum).to eq(legacy_checksum) + expect(job.job_artifacts_trace.file.exists?).to be_truthy + expect(job.job_artifacts_trace.file.filename).to eq('job.log') end - context 'when the job has already had trace artifact' do + context 'when failed to create trace artifact record' do before do - create(:ci_job_artifact, :trace, job: job) + # When ActiveRecord error happens + allow_any_instance_of(Ci::JobArtifact).to receive(:save).and_return(false) + allow_any_instance_of(Ci::JobArtifact).to receive_message_chain(:errors, :full_messages) + .and_return("Error") + + subject rescue nil + + job.reload end - it 'does not create trace artifact' do - expect { subject }.not_to change { Ci::JobArtifact.count } + it 'keeps legacy trace and removes trace artifact' do + expect(File.exist?(legacy_path)).to be_truthy + expect(job.job_artifacts_trace).to be_nil end end end context 'when the job does not have a trace file' do + let!(:job) { create(:ci_build) } + it 'does not create trace artifact' do expect { subject }.not_to change { Ci::JobArtifact.count } end end end + + context 'when the job has already had trace artifact' do + let!(:job) { create(:ci_build, :trace_artifact) } + + it 'does not create trace artifact' do + expect { subject }.not_to change { Ci::JobArtifact.count } + end + end end end diff --git a/spec/services/labels/find_or_create_service_spec.rb b/spec/services/labels/find_or_create_service_spec.rb index 78aa5d442e7..68d5660445a 100644 --- a/spec/services/labels/find_or_create_service_spec.rb +++ b/spec/services/labels/find_or_create_service_spec.rb @@ -15,47 +15,79 @@ describe Labels::FindOrCreateService do context 'when acting on behalf of a specific user' do let(:user) { create(:user) } - subject(:service) { described_class.new(user, project, params) } - before do - project.add_developer(user) - end - context 'when label does not exist at group level' do - it 'creates a new label at project level' do - expect { service.execute }.to change(project.labels, :count).by(1) + context 'when finding labels on project level' do + subject(:service) { described_class.new(user, project, params) } + + before do + project.add_developer(user) end - end - context 'when label exists at group level' do - it 'returns the group label' do - group_label = create(:group_label, group: group, title: 'Security') + context 'when label does not exist at group level' do + it 'creates a new label at project level' do + expect { service.execute }.to change(project.labels, :count).by(1) + end + end - expect(service.execute).to eq group_label + context 'when label exists at group level' do + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') + + expect(service.execute).to eq group_label + end + end + + context 'when label exists at project level' do + it 'returns the project label' do + project_label = create(:label, project: project, title: 'Security') + + expect(service.execute).to eq project_label + end end end - context 'when label does not exist at group level' do - it 'creates a new label at project leve' do - expect { service.execute }.to change(project.labels, :count).by(1) + context 'when finding labels on group level' do + subject(:service) { described_class.new(user, group, params) } + + before do + group.add_developer(user) + end + + context 'when label does not exist at group level' do + it 'creates a new label at group level' do + expect { service.execute }.to change(group.labels, :count).by(1) + end + end + + context 'when label exists at group level' do + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') + + expect(service.execute).to eq group_label + end end end + end + + context 'when authorization is not required' do + context 'when finding labels on project level' do + subject(:service) { described_class.new(nil, project, params) } - context 'when label exists at project level' do it 'returns the project label' do project_label = create(:label, project: project, title: 'Security') - expect(service.execute).to eq project_label + expect(service.execute(skip_authorization: true)).to eq project_label end end - end - context 'when authorization is not required' do - subject(:service) { described_class.new(nil, project, params) } + context 'when finding labels on group level' do + subject(:service) { described_class.new(nil, group, params) } - it 'returns the project label' do - project_label = create(:label, project: project, title: 'Security') + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') - expect(service.execute(skip_authorization: true)).to eq project_label + expect(service.execute(skip_authorization: true)).to eq group_label + end end end end diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 5189c57b7db..8603b7f3e2c 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -78,8 +78,10 @@ RSpec.configure do |config| end config.after(:example, :js) do |example| - # prevent localstorage from introducing side effects based on test order - execute_script("localStorage.clear();") + # prevent localStorage from introducing side effects based on test order + unless ['', 'about:blank', 'data:,'].include? Capybara.current_session.driver.browser.current_url + execute_script("localStorage.clear();") + end # capybara/rspec already calls Capybara.reset_sessions! in an `after` hook, # but `block_and_wait_for_requests_complete` is called before it so by diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 1809ae1d141..5edc5de2a09 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -28,11 +28,11 @@ RSpec.configure do |config| end config.before(:each, :js) do - DatabaseCleaner.strategy = :deletion + DatabaseCleaner.strategy = :deletion, { cache_tables: false } end config.before(:each, :delete) do - DatabaseCleaner.strategy = :deletion + DatabaseCleaner.strategy = :deletion, { cache_tables: false } end config.before(:each, :migration) do diff --git a/spec/workers/authorized_projects_worker_spec.rb b/spec/workers/authorized_projects_worker_spec.rb index 0d6eb536c33..d095138f6b7 100644 --- a/spec/workers/authorized_projects_worker_spec.rb +++ b/spec/workers/authorized_projects_worker_spec.rb @@ -1,79 +1,6 @@ require 'spec_helper' describe AuthorizedProjectsWorker do - let(:project) { create(:project) } - - def build_args_list(*ids, multiply: 1) - args_list = ids.map { |id| [id] } - args_list * multiply - end - - describe '.bulk_perform_and_wait' do - it 'schedules the ids and waits for the jobs to complete' do - args_list = build_args_list(project.owner.id) - - project.owner.project_authorizations.delete_all - described_class.bulk_perform_and_wait(args_list) - - expect(project.owner.project_authorizations.count).to eq(1) - end - - it 'inlines workloads <= 3 jobs' do - args_list = build_args_list(project.owner.id, multiply: 3) - expect(described_class).to receive(:bulk_perform_inline).with(args_list) - - described_class.bulk_perform_and_wait(args_list) - end - - it 'runs > 3 jobs using sidekiq' do - project.owner.project_authorizations.delete_all - - expect(described_class).to receive(:bulk_perform_async).and_call_original - - args_list = build_args_list(project.owner.id, multiply: 4) - described_class.bulk_perform_and_wait(args_list) - - expect(project.owner.project_authorizations.count).to eq(1) - end - end - - describe '.bulk_perform_inline' do - it 'refreshes the authorizations inline' do - project.owner.project_authorizations.delete_all - - expect_any_instance_of(described_class).to receive(:perform).and_call_original - - described_class.bulk_perform_inline(build_args_list(project.owner.id)) - - expect(project.owner.project_authorizations.count).to eq(1) - end - - it 'enqueues jobs if an error is raised' do - invalid_id = -1 - args_list = build_args_list(project.owner.id, invalid_id) - - allow_any_instance_of(described_class).to receive(:perform).with(project.owner.id) - allow_any_instance_of(described_class).to receive(:perform).with(invalid_id).and_raise(ArgumentError) - expect(described_class).to receive(:bulk_perform_async).with(build_args_list(invalid_id)) - - described_class.bulk_perform_inline(args_list) - end - end - - describe '.bulk_perform_async' do - it "uses it's respective sidekiq queue" do - args_list = build_args_list(project.owner.id) - push_bulk_args = { - 'class' => described_class, - 'args' => args_list - } - - expect(Sidekiq::Client).to receive(:push_bulk).with(push_bulk_args).once - - described_class.bulk_perform_async(args_list) - end - end - describe '#perform' do let(:user) { create(:user) } @@ -85,12 +12,6 @@ describe AuthorizedProjectsWorker do job.perform(user.id) end - it 'notifies the JobWaiter when done if the key is provided' do - expect(Gitlab::JobWaiter).to receive(:notify).with('notify-key', job.jid) - - job.perform(user.id, 'notify-key') - end - context "when the user is not found" do it "does nothing" do expect_any_instance_of(User).not_to receive(:refresh_authorized_projects) diff --git a/spec/workers/concerns/waitable_worker_spec.rb b/spec/workers/concerns/waitable_worker_spec.rb new file mode 100644 index 00000000000..4af0de86ac9 --- /dev/null +++ b/spec/workers/concerns/waitable_worker_spec.rb @@ -0,0 +1,92 @@ +require 'spec_helper' + +describe WaitableWorker do + let(:worker) do + Class.new do + def self.name + 'Gitlab::Foo::Bar::DummyWorker' + end + + class << self + cattr_accessor(:counter) { 0 } + end + + include ApplicationWorker + prepend WaitableWorker + + def perform(i = 0) + self.class.counter += i + end + end + end + + subject(:job) { worker.new } + + describe '.bulk_perform_and_wait' do + it 'schedules the jobs and waits for them to complete' do + worker.bulk_perform_and_wait([[1], [2]]) + + expect(worker.counter).to eq(3) + end + + it 'inlines workloads <= 3 jobs' do + args_list = [[1], [2], [3]] + expect(worker).to receive(:bulk_perform_inline).with(args_list).and_call_original + + worker.bulk_perform_and_wait(args_list) + + expect(worker.counter).to eq(6) + end + + it 'runs > 3 jobs using sidekiq' do + expect(worker).to receive(:bulk_perform_async) + + worker.bulk_perform_and_wait([[1], [2], [3], [4]]) + end + end + + describe '.bulk_perform_inline' do + it 'runs the jobs inline' do + expect(worker).not_to receive(:bulk_perform_async) + + worker.bulk_perform_inline([[1], [2]]) + + expect(worker.counter).to eq(3) + end + + it 'enqueues jobs if an error is raised' do + expect(worker).to receive(:bulk_perform_async).with([['foo']]) + + worker.bulk_perform_inline([[1], ['foo']]) + end + end + + describe '#perform' do + shared_examples 'perform' do + it 'notifies the JobWaiter when done if the key is provided' do + key = Gitlab::JobWaiter.new.key + expect(Gitlab::JobWaiter).to receive(:notify).with(key, job.jid) + + job.perform(*args, key) + end + + it 'does not notify the JobWaiter when done if no key is provided' do + expect(Gitlab::JobWaiter).not_to receive(:notify) + + job.perform(*args) + end + end + + context 'when the worker takes arguments' do + let(:args) { [1] } + + it_behaves_like 'perform' + end + + context 'when the worker takes no arguments' do + let(:args) { [] } + + it_behaves_like 'perform' + end + end +end |