diff options
| author | Mike Greiling <mgreiling@gitlab.com> | 2017-08-10 03:14:21 +0000 |
|---|---|---|
| committer | Mike Greiling <mgreiling@gitlab.com> | 2017-08-10 03:14:21 +0000 |
| commit | 7e5ac7114ca5540c5a9b6b6cd0f832c5f71794d2 (patch) | |
| tree | 57b572f23b16f4b00159af1759d9d59cb14c1e5b | |
| parent | 3d08e47239ffd00856507b73f1ce36ffa1e05256 (diff) | |
| parent | 2925850ceec3ef89eb1f60b0a648dbc5f72d8683 (diff) | |
| download | gitlab-ce-36133-project-overview-page-doesn-t-load-commit-metadata-for-repo-view.tar.gz | |
Merge branch 'master' into '36133-project-overview-page-doesn-t-load-commit-metadata-for-repo-view'36133-project-overview-page-doesn-t-load-commit-metadata-for-repo-view
# Conflicts:
# app/assets/javascripts/dispatcher.js
218 files changed, 1763 insertions, 778 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f55bdc19eec..e10958b3bee 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -514,8 +514,11 @@ codeclimate: services: - docker:dind script: + - cp .rubocop.yml .rubocop.yml.bak + - grep -v "rubocop-gitlab-security" .rubocop.yml.bak > .rubocop.yml - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate analyze -f json > raw_codeclimate.json - cat raw_codeclimate.json | docker run -i stedolan/jq -c 'map({check_name,fingerprint,location})' > codeclimate.json + - mv .rubocop.yml.bak .rubocop.yml artifacts: paths: [codeclimate.json] diff --git a/.rubocop.yml b/.rubocop.yml index a5ccec0437b..84e4a3c2e49 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,6 @@ require: - rubocop-rspec + - rubocop-gitlab-security - ./rubocop/rubocop inherit_from: .rubocop_todo.yml @@ -206,6 +207,13 @@ Layout/SpaceAroundKeyword: Layout/SpaceAroundOperators: Enabled: true +# Checks that block braces have or don't have a space before the opening +# brace depending on configuration. +# Configuration parameters: EnforcedStyle, SupportedStyles. +# SupportedStyles: space, no_space +Layout/SpaceBeforeBlockBraces: + Enabled: true + # No spaces before commas. Layout/SpaceBeforeComma: Enabled: true @@ -1156,3 +1164,35 @@ RSpec/SubjectStub: # Prefer using verifying doubles over normal doubles. RSpec/VerifiedDoubles: Enabled: false + +# GitlabSecurity ############################################################## + +GitlabSecurity/DeepMunge: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' + +GitlabSecurity/PublicSend: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' + +GitlabSecurity/RedirectToParamsUpdate: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' + +GitlabSecurity/SqlInjection: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' + +GitlabSecurity/SystemCommandInjection: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 9caef3bde08..78ab7f7204f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -26,13 +26,6 @@ Layout/IndentArray: Layout/IndentHash: Enabled: false -# Offense count: 174 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, SupportedStyles. -# SupportedStyles: space, no_space -Layout/SpaceBeforeBlockBraces: - Enabled: false - # Offense count: 8 # Cop supports --auto-correct. # Configuration parameters: AllowForAlignment. @@ -341,6 +341,7 @@ group :development, :test do gem 'rubocop', '~> 0.49.1', require: false gem 'rubocop-rspec', '~> 1.15.1', require: false + gem 'rubocop-gitlab-security', '~> 0.0.6', require: false gem 'scss_lint', '~> 0.54.0', require: false gem 'haml_lint', '~> 0.26.0', require: false gem 'simplecov', '~> 0.14.0', require: false @@ -354,7 +355,7 @@ group :development, :test do gem 'activerecord_sane_schema_dumper', '0.2' - gem 'stackprof', '~> 0.2.10' + gem 'stackprof', '~> 0.2.10', require: false end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 948ba02a72c..b865ff6ee32 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -548,7 +548,7 @@ GEM rubypants (~> 0.2) orm_adapter (0.5.0) os (0.9.6) - parallel (1.11.2) + parallel (1.12.0) paranoia (2.3.1) activerecord (>= 4.0, < 5.2) parser (2.4.0.0) @@ -741,8 +741,9 @@ GEM rainbow (>= 1.99.1, < 3.0) ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) + rubocop-gitlab-security (0.0.6) + rubocop (>= 0.47.1) rubocop-rspec (1.15.1) - rubocop (>= 0.42.0) ruby-fogbugz (0.2.1) crack (~> 0.4) ruby-prof (0.16.2) @@ -1088,6 +1089,7 @@ DEPENDENCIES rspec-set (~> 0.1.3) rspec_profiling (~> 0.0.5) rubocop (~> 0.49.1) + rubocop-gitlab-security (~> 0.0.6) rubocop-rspec (~> 1.15.1) ruby-fogbugz (~> 0.2.1) ruby-prof (~> 0.16.2) diff --git a/PROCESS.md b/PROCESS.md index 2b3d142bf77..e5b17784d20 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -119,6 +119,12 @@ only be left until after the freeze if: are aware of it. * It is in the correct milestone, with the ~Deliverable label. +If a merge request is not ready, but the developers and Product Manager +responsible for the feature think it is essential that it is in the release, +they can [ask for an exception](#asking-for-an-exception) in advance. This is +preferable to merging something that we are not confident in, but should still +be a rare case: most features can be allowed to slip a release. + All Community Edition merge requests from GitLab team members merged on the freeze date (the 7th) should have a corresponding Enterprise Edition merge request, even if there are no conflicts. This is to reduce the size of the @@ -128,11 +134,26 @@ information, see ### After the 7th -Once the stable branch is frozen, only fixes for [regressions](#regressions) -and security issues will be cherry-picked into the stable branch. -Any merge requests cherry-picked into the stable branch for a previous release will also be picked into the latest stable branch. -These fixes will be shipped in the next RC for that release if it is before the 22nd. -If the fixes are are completed on or after the 22nd, they will be shipped in a patch for that release. +Once the stable branch is frozen, the only MRs that can be cherry-picked into +the stable branch are: + +* Fixes for [regressions](#regressions) +* Fixes for security issues +* New or updated translations (as long as they do not touch application code) + +Any merge requests cherry-picked into the stable branch for a previous release +will also be picked into the latest stable branch. These fixes will be shipped +in the next RC for that release if it is before the 22nd. If the fixes are are +completed on or after the 22nd, they will be shipped in a patch for that +release. + +During the feature freeze all merge requests that are meant to go into the upcoming +release should have the correct milestone assigned _and_ have the label +~"Pick into Stable" set, so that release managers can find and pick them. +Merge requests without a milestone and this label will +not be merged into any stable branches. + +### Asking for an exception If you think a merge request should go into an RC or patch even though it does not meet these requirements, you can ask for an exception to be made. Exceptions require sign-off from 3 people besides the developer: @@ -152,11 +173,7 @@ When in doubt, we err on the side of _not_ cherry-picking. For example, it is likely that an exception will be made for a trivial 1-5 line performance improvement (e.g. adding a database index or adding `includes` to a query), but not for a new feature, no matter how relatively small or thoroughly tested. -During the feature freeze all merge requests that are meant to go into the upcoming -release should have the correct milestone assigned _and_ have the label -~"Pick into Stable" set, so that release managers can find and pick them. -Merge requests without a milestone and this label will -not be merged into any stable branches. +All MRs which have had exceptions granted must be merged by the 15th. ### Regressions diff --git a/app/assets/javascripts/blob/blob_file_dropzone.js b/app/assets/javascripts/blob/blob_file_dropzone.js index dc636050221..26d3419a162 100644 --- a/app/assets/javascripts/blob/blob_file_dropzone.js +++ b/app/assets/javascripts/blob/blob_file_dropzone.js @@ -1,9 +1,24 @@ /* eslint-disable func-names, object-shorthand, prefer-arrow-callback */ /* global Dropzone */ +import '../lib/utils/url_utility'; +import { HIDDEN_CLASS } from '../lib/utils/constants'; + +function toggleLoading($el, $icon, loading) { + if (loading) { + $el.disable(); + $icon.removeClass(HIDDEN_CLASS); + } else { + $el.enable(); + $icon.addClass(HIDDEN_CLASS); + } +} export default class BlobFileDropzone { constructor(form, method) { const formDropzone = form.find('.dropzone'); + const submitButton = form.find('#submit-all'); + const submitButtonLoadingIcon = submitButton.find('.js-loading-icon'); + const dropzoneMessage = form.find('.dz-message'); Dropzone.autoDiscover = false; const dropzone = formDropzone.dropzone({ @@ -26,12 +41,20 @@ export default class BlobFileDropzone { }, init: function () { this.on('addedfile', function () { + toggleLoading(submitButton, submitButtonLoadingIcon, false); + dropzoneMessage.addClass(HIDDEN_CLASS); $('.dropzone-alerts').html('').hide(); }); + this.on('removedfile', function () { + toggleLoading(submitButton, submitButtonLoadingIcon, false); + dropzoneMessage.removeClass(HIDDEN_CLASS); + }); this.on('success', function (header, response) { - window.location.href = response.filePath; + $('#modal-upload-blob').modal('hide'); + window.gl.utils.visitUrl(response.filePath); }); this.on('maxfilesexceeded', function (file) { + dropzoneMessage.addClass(HIDDEN_CLASS); this.removeFile(file); }); this.on('sending', function (file, xhr, formData) { @@ -48,14 +71,15 @@ export default class BlobFileDropzone { }, }); - const submitButton = form.find('#submit-all')[0]; - submitButton.addEventListener('click', function (e) { + submitButton.on('click', (e) => { e.preventDefault(); e.stopPropagation(); if (dropzone[0].dropzone.getQueuedFiles().length === 0) { // eslint-disable-next-line no-alert alert('Please select a file'); + return false; } + toggleLoading(submitButton, submitButtonLoadingIcon, true); dropzone[0].dropzone.processQueue(); return false; }); diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 745c3e892f3..482825f5947 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -343,6 +343,7 @@ import UserFeatureHelper from './helpers/user_feature_helper'; if ($('#tree-slider').length) new TreeView(); if ($('.blob-viewer').length) new BlobViewer(); + if ($('.project-show-activity').length) new gl.Activities(); $('#tree-slider').waitForImages(function() { gl.utils.ajaxGet(document.querySelector('.js-tree-content').dataset.logsPath); }); diff --git a/app/assets/javascripts/due_date_select.js b/app/assets/javascripts/due_date_select.js index 2856c8e2862..ee71728184f 100644 --- a/app/assets/javascripts/due_date_select.js +++ b/app/assets/javascripts/due_date_select.js @@ -1,7 +1,7 @@ /* eslint-disable wrap-iife, func-names, space-before-function-paren, comma-dangle, prefer-template, consistent-return, class-methods-use-this, arrow-body-style, no-unused-vars, no-underscore-dangle, no-new, max-len, no-sequences, no-unused-expressions, no-param-reassign */ /* global dateFormat */ -/* global Pikaday */ +import Pikaday from 'pikaday'; import DateFix from './lib/utils/datefix'; class DueDateSelect { diff --git a/app/assets/javascripts/fly_out_nav.js b/app/assets/javascripts/fly_out_nav.js index 301e82f4610..aabea56408a 100644 --- a/app/assets/javascripts/fly_out_nav.js +++ b/app/assets/javascripts/fly_out_nav.js @@ -1,7 +1,17 @@ /* global bp */ +import Cookies from 'js-cookie'; import './breakpoints'; -export const canShowSubItems = () => bp.getBreakpointSize() === 'md' || bp.getBreakpointSize() === 'lg'; +export const canShowActiveSubItems = (el) => { + const isHiddenByMedia = bp.getBreakpointSize() === 'sm' || bp.getBreakpointSize() === 'md'; + + if (el.classList.contains('active') && !isHiddenByMedia) { + return Cookies.get('sidebar_collapsed') === 'true'; + } + + return true; +}; +export const canShowSubItems = () => bp.getBreakpointSize() === 'sm' || bp.getBreakpointSize() === 'md' || bp.getBreakpointSize() === 'lg'; export const calculateTop = (boundingRect, outerHeight) => { const windowHeight = window.innerHeight; @@ -14,9 +24,10 @@ export const calculateTop = (boundingRect, outerHeight) => { export const showSubLevelItems = (el) => { const subItems = el.querySelector('.sidebar-sub-level-items'); - if (!subItems || !canShowSubItems()) return; + if (!subItems || !canShowSubItems() || !canShowActiveSubItems(el)) return; subItems.style.display = 'block'; + el.classList.add('is-showing-fly-out'); el.classList.add('is-over'); const boundingRect = el.getBoundingClientRect(); @@ -34,15 +45,16 @@ export const showSubLevelItems = (el) => { export const hideSubLevelItems = (el) => { const subItems = el.querySelector('.sidebar-sub-level-items'); - if (!subItems || !canShowSubItems()) return; + if (!subItems || !canShowSubItems() || !canShowActiveSubItems(el)) return; + el.classList.remove('is-showing-fly-out'); el.classList.remove('is-over'); subItems.style.display = 'none'; subItems.classList.remove('is-above'); }; export default () => { - const items = [...document.querySelectorAll('.sidebar-top-level-items > li:not(.active)')] + const items = [...document.querySelectorAll('.sidebar-top-level-items > li')] .filter(el => el.querySelector('.sidebar-sub-level-items')); items.forEach((el) => { diff --git a/app/assets/javascripts/graphs/graphs_charts.js b/app/assets/javascripts/graphs/graphs_charts.js index 279ffef770f..ec6eab34989 100644 --- a/app/assets/javascripts/graphs/graphs_charts.js +++ b/app/assets/javascripts/graphs/graphs_charts.js @@ -1,4 +1,5 @@ import Chart from 'vendor/Chart'; +import _ from 'underscore'; document.addEventListener('DOMContentLoaded', () => { const projectChartData = JSON.parse(document.getElementById('projectChartData').innerHTML); @@ -27,28 +28,25 @@ document.addEventListener('DOMContentLoaded', () => { return generateChart(); }; - const chartData = (keys, values) => { - const data = { - labels: keys, - datasets: [{ - fillColor: 'rgba(220,220,220,0.5)', - strokeColor: 'rgba(220,220,220,1)', - barStrokeWidth: 1, - barValueSpacing: 1, - barDatasetSpacing: 1, - data: values, - }], - }; - return data; - }; - - const hourData = chartData(projectChartData.hour.keys, projectChartData.hour.values); + const chartData = data => ({ + labels: Object.keys(data), + datasets: [{ + fillColor: 'rgba(220,220,220,0.5)', + strokeColor: 'rgba(220,220,220,1)', + barStrokeWidth: 1, + barValueSpacing: 1, + barDatasetSpacing: 1, + data: _.values(data), + }], + }); + + const hourData = chartData(projectChartData.hour); responsiveChart($('#hour-chart'), hourData); - const dayData = chartData(projectChartData.weekDays.keys, projectChartData.weekDays.values); + const dayData = chartData(projectChartData.weekDays); responsiveChart($('#weekday-chart'), dayData); - const monthData = chartData(projectChartData.month.keys, projectChartData.month.values); + const monthData = chartData(projectChartData.month); responsiveChart($('#month-chart'), monthData); const data = projectChartData.languages; diff --git a/app/assets/javascripts/issuable_form.js b/app/assets/javascripts/issuable_form.js index 9ac1325fc95..3f848e0859b 100644 --- a/app/assets/javascripts/issuable_form.js +++ b/app/assets/javascripts/issuable_form.js @@ -2,8 +2,8 @@ /* global GitLab */ /* global Autosave */ /* global dateFormat */ -/* global Pikaday */ +import Pikaday from 'pikaday'; import UsersSelect from './users_select'; import GfmAutoComplete from './gfm_auto_complete'; import ZenMode from './zen_mode'; diff --git a/app/assets/javascripts/lib/utils/constants.js b/app/assets/javascripts/lib/utils/constants.js index 1e96c7ab5cd..7a72509d234 100644 --- a/app/assets/javascripts/lib/utils/constants.js +++ b/app/assets/javascripts/lib/utils/constants.js @@ -1,2 +1,3 @@ /* eslint-disable import/prefer-default-export */ export const BYTES_IN_KIB = 1024; +export const HIDDEN_CLASS = 'hidden'; diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 42092a34c2f..e0c61a474c6 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -7,7 +7,6 @@ import jQuery from 'jquery'; import _ from 'underscore'; import Cookies from 'js-cookie'; -import Pikaday from 'pikaday'; import Dropzone from 'dropzone'; import Sortable from 'vendor/Sortable'; @@ -20,7 +19,6 @@ import 'vendor/fuzzaldrin-plus'; window.jQuery = jQuery; window.$ = jQuery; window._ = _; -window.Pikaday = Pikaday; window.Dropzone = Dropzone; window.Sortable = Sortable; diff --git a/app/assets/javascripts/member_expiration_date.js b/app/assets/javascripts/member_expiration_date.js index e034729bd39..cc9016e74da 100644 --- a/app/assets/javascripts/member_expiration_date.js +++ b/app/assets/javascripts/member_expiration_date.js @@ -1,5 +1,7 @@ -/* global Pikaday */ /* global dateFormat */ + +import Pikaday from 'pikaday'; + (() => { // Add datepickers to all `js-access-expiration-date` elements. If those elements are // children of an element with the `clearable-input` class, and have a sibling diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 5e374360359..293aa194528 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -372,6 +372,10 @@ table { background: $gl-success !important; } +.dz-message { + margin: 0; +} + .space-right { margin-right: 10px; } diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index d9f92e93160..b677882eba4 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -26,7 +26,7 @@ header { &.navbar-gitlab { padding: 0 16px; - z-index: 2000; + z-index: 1000; margin-bottom: 0; min-height: $header-height; background-color: $gray-light; diff --git a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss index b666223b120..4c35e3a9c3c 100644 --- a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss +++ b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss @@ -162,3 +162,5 @@ $pre-color: $gl-text-color !default; $pre-border-color: $border-color; $table-bg-accent: $gray-light; + +$zindex-popover: 900; diff --git a/app/assets/stylesheets/new_sidebar.scss b/app/assets/stylesheets/new_sidebar.scss index 76dccd2df56..4367b8c1a15 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -104,11 +104,14 @@ $new-sidebar-collapsed-width: 50px; &.sidebar-icons-only { width: $new-sidebar-collapsed-width; - .nav-item-name, .badge, .project-title { display: none; } + + .nav-item-name { + opacity: 0; + } } &.nav-sidebar-expanded { @@ -182,7 +185,7 @@ $new-sidebar-collapsed-width: 50px; > li { a { - padding: 8px 16px 8px 50px; + padding: 8px 16px 8px 40px; &:hover, &:focus { @@ -215,12 +218,15 @@ $new-sidebar-collapsed-width: 50px; &:hover { color: $gl-text-color; + + svg { + fill: $gl-text-color; + } } } - &:not(.active) { + &.is-showing-fly-out { > a { - margin-left: 1px; margin-right: 2px; } @@ -229,7 +235,7 @@ $new-sidebar-collapsed-width: 50px; position: fixed; top: 0; left: $new-sidebar-width; - width: 150px; + min-width: 150px; margin-top: -1px; padding: 8px 1px; background-color: $white-light; @@ -271,6 +277,14 @@ $new-sidebar-collapsed-width: 50px; } } + > .active { + box-shadow: none; + + > a { + background-color: transparent; + } + } + a { padding: 8px 16px; color: $gl-text-color; @@ -294,6 +308,7 @@ $new-sidebar-collapsed-width: 50px; > a { margin-left: 4px; + padding-left: 12px; } .badge { @@ -354,7 +369,7 @@ $new-sidebar-collapsed-width: 50px; .sidebar-icons-only { .context-header { - height: 60px; + height: 61px; a { padding: 10px 4px; diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index b78db402c13..d14b976374c 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -35,7 +35,7 @@ .commit-box, .info-well, .commit-ci-menu, - .files-changed, + .files-changed-inner, .limited-header-width, .limited-width-notes { @extend .fixed-width-container; diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index a8e0f251cd3..11236cbf2e7 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -216,6 +216,9 @@ } .blob-upload-dropzone-previews { + display: flex; + justify-content: center; + align-items: center; text-align: center; border: 2px; border-style: dashed; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5b448008a1b..1d92ea11bda 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -117,7 +117,7 @@ class ApplicationController < ActionController::Base Raven.capture_exception(exception) if sentry_enabled? application_trace = ActionDispatch::ExceptionWrapper.new(env, exception).application_trace - application_trace.map!{ |t| " #{t}\n" } + application_trace.map! { |t| " #{t}\n" } logger.error "\n#{exception.class.name} (#{exception.message}):\n#{application_trace.join}" end diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index 53a5981e564..baa6645e5ce 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -68,15 +68,15 @@ class Import::GithubController < Import::BaseController end def new_import_url - public_send("new_import_#{provider}_url") + public_send("new_import_#{provider}_url") # rubocop:disable GitlabSecurity/PublicSend end def status_import_url - public_send("status_import_#{provider}_url") + public_send("status_import_#{provider}_url") # rubocop:disable GitlabSecurity/PublicSend end def callback_import_url - public_send("callback_import_#{provider}_url") + public_send("callback_import_#{provider}_url") # rubocop:disable GitlabSecurity/PublicSend end def provider_unauthorized diff --git a/app/controllers/import/gitlab_controller.rb b/app/controllers/import/gitlab_controller.rb index 73837ffbe67..407154e59a0 100644 --- a/app/controllers/import/gitlab_controller.rb +++ b/app/controllers/import/gitlab_controller.rb @@ -15,7 +15,7 @@ class Import::GitlabController < Import::BaseController @already_added_projects = current_user.created_projects.where(import_type: "gitlab") already_added_projects_names = @already_added_projects.pluck(:import_source) - @repos = @repos.to_a.reject{ |repo| already_added_projects_names.include? repo["path_with_namespace"] } + @repos = @repos.to_a.reject { |repo| already_added_projects_names.include? repo["path_with_namespace"] } end def jobs diff --git a/app/helpers/graph_helper.rb b/app/helpers/graph_helper.rb index 2e9b72e9613..c53ea4519da 100644 --- a/app/helpers/graph_helper.rb +++ b/app/helpers/graph_helper.rb @@ -3,7 +3,7 @@ module GraphHelper refs = "" # Commit::ref_names already strips the refs/XXX from important refs (e.g. refs/heads/XXX) # so anything leftover is internally used by GitLab - commit_refs = commit.ref_names(repo).reject{ |name| name.starts_with?('refs/') } + commit_refs = commit.ref_names(repo).reject { |name| name.starts_with?('refs/') } refs << commit_refs.join(' ') # append note count diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index f4fad7150e8..70ea35fab1e 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -151,7 +151,7 @@ module IssuablesHelper end def issuable_labels_tooltip(labels, limit: 5) - first, last = labels.partition.with_index{ |_, i| i < limit } + first, last = labels.partition.with_index { |_, i| i < limit } label_names = first.collect(&:name) label_names << "and #{last.size} more" unless last.empty? @@ -234,7 +234,7 @@ module IssuablesHelper end def issuables_count_for_state(issuable_type, state, finder: nil) - finder ||= public_send("#{issuable_type}_finder") + finder ||= public_send("#{issuable_type}_finder") # rubocop:disable GitlabSecurity/PublicSend cache_key = finder.state_counter_cache_key @counts ||= {} @@ -329,7 +329,7 @@ module IssuablesHelper end def selected_template(issuable) - params[:issuable_template] if issuable_templates(issuable).any?{ |template| template[:name] == params[:issuable_template] } + params[:issuable_template] if issuable_templates(issuable).any? { |template| template[:name] == params[:issuable_template] } end def issuable_todo_button_data(issuable, todo, is_collapsed) diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 4b99de1b6a5..e60513b35c7 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -43,11 +43,11 @@ module LabelsHelper def label_filter_path(subject, label, type: :issue) case subject when Group - send("#{type.to_s.pluralize}_group_path", + send("#{type.to_s.pluralize}_group_path", # rubocop:disable GitlabSecurity/PublicSend subject, label_name: [label.name]) when Project - send("namespace_project_#{type.to_s.pluralize}_path", + send("namespace_project_#{type.to_s.pluralize}_path", # rubocop:disable GitlabSecurity/PublicSend subject.namespace, subject, label_name: [label.name]) diff --git a/app/models/commit.rb b/app/models/commit.rb index 7940733f557..638fddc5d3d 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -55,7 +55,8 @@ class Commit end def from_hash(hash, project) - new(Gitlab::Git::Commit.new(hash), project) + raw_commit = Gitlab::Git::Commit.new(project.repository.raw, hash) + new(raw_commit, project) end def valid_hash?(key) @@ -320,21 +321,11 @@ class Commit end def raw_diffs(*args) - if Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs) - Gitlab::GitalyClient::CommitService.new(project.repository).diff_from_parent(self, *args) - else - raw.diffs(*args) - end + raw.diffs(*args) end def raw_deltas - @deltas ||= Gitlab::GitalyClient.migrate(:commit_deltas) do |is_enabled| - if is_enabled - Gitlab::GitalyClient::CommitService.new(project.repository).commit_deltas(self) - else - raw.deltas - end - end + @deltas ||= raw.deltas end def diffs(diff_options = nil) diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb index bd75f25a210..f2707022a4b 100644 --- a/app/models/concerns/spammable.rb +++ b/app/models/concerns/spammable.rb @@ -58,7 +58,7 @@ module Spammable options.fetch(:spam_title, false) end - public_send(attr.first) if attr && respond_to?(attr.first.to_sym) + public_send(attr.first) if attr && respond_to?(attr.first.to_sym) # rubocop:disable GitlabSecurity/PublicSend end def spam_description @@ -66,12 +66,12 @@ module Spammable options.fetch(:spam_description, false) end - public_send(attr.first) if attr && respond_to?(attr.first.to_sym) + public_send(attr.first) if attr && respond_to?(attr.first.to_sym) # rubocop:disable GitlabSecurity/PublicSend end def spammable_text result = self.class.spammable_attrs.map do |attr| - public_send(attr.first) + public_send(attr.first) # rubocop:disable GitlabSecurity/PublicSend end result.reject(&:blank?).join("\n") diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb index 1ca7f91dc03..a7d5de48c66 100644 --- a/app/models/concerns/token_authenticatable.rb +++ b/app/models/concerns/token_authenticatable.rb @@ -44,7 +44,8 @@ module TokenAuthenticatable end define_method("ensure_#{token_field}!") do - send("reset_#{token_field}!") if read_attribute(token_field).blank? + send("reset_#{token_field}!") if read_attribute(token_field).blank? # rubocop:disable GitlabSecurity/PublicSend + read_attribute(token_field) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index e83b11f7668..f90194041b1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -162,7 +162,7 @@ class MergeRequest < ActiveRecord::Base target = unscoped.where(target_project_id: relation).select(:id) union = Gitlab::SQL::Union.new([source, target]) - where("merge_requests.id IN (#{union.to_sql})") + where("merge_requests.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index d9d746ccf41..58050e1f438 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -282,9 +282,7 @@ class MergeRequestDiff < ActiveRecord::Base def load_commits commits = st_commits.presence || merge_request_diff_commits - commits.map do |commit| - Commit.new(Gitlab::Git::Commit.new(commit.to_hash), merge_request.source_project) - end + commits.map { |commit| Commit.from_hash(commit.to_hash, project) } end def save_diffs diff --git a/app/models/merge_request_diff_commit.rb b/app/models/merge_request_diff_commit.rb index cafdbe11849..670b26d4ca3 100644 --- a/app/models/merge_request_diff_commit.rb +++ b/app/models/merge_request_diff_commit.rb @@ -26,7 +26,7 @@ class MergeRequestDiffCommit < ActiveRecord::Base def to_hash Gitlab::Git::Commit::SERIALIZE_KEYS.each_with_object({}) do |key, hash| - hash[key] = public_send(key) + hash[key] = public_send(key) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb index 2bc00a082df..0e5acb22d50 100644 --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb @@ -206,7 +206,7 @@ module Network # Visit branching chains leaves.each do |l| - parents = l.parents(@map).select{|p| p.space.zero?} + parents = l.parents(@map).select {|p| p.space.zero?} parents.each do |p| place_chain(p, l.time) end diff --git a/app/models/note.rb b/app/models/note.rb index d0e3bc0bfed..a752c897d63 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -77,20 +77,20 @@ class Note < ActiveRecord::Base # Scopes scope :for_commit_id, ->(commit_id) { where(noteable_type: "Commit", commit_id: commit_id) } - scope :system, ->{ where(system: true) } - scope :user, ->{ where(system: false) } - scope :common, ->{ where(noteable_type: ["", nil]) } - scope :fresh, ->{ order(created_at: :asc, id: :asc) } - scope :updated_after, ->(time){ where('updated_at > ?', time) } - scope :inc_author_project, ->{ includes(:project, :author) } - scope :inc_author, ->{ includes(:author) } + scope :system, -> { where(system: true) } + scope :user, -> { where(system: false) } + scope :common, -> { where(noteable_type: ["", nil]) } + scope :fresh, -> { order(created_at: :asc, id: :asc) } + scope :updated_after, ->(time) { where('updated_at > ?', time) } + scope :inc_author_project, -> { includes(:project, :author) } + scope :inc_author, -> { includes(:author) } scope :inc_relations_for_view, -> do includes(:project, :author, :updated_by, :resolved_by, :award_emoji, :system_note_metadata) end - scope :diff_notes, ->{ where(type: %w(LegacyDiffNote DiffNote)) } - scope :new_diff_notes, ->{ where(type: 'DiffNote') } - scope :non_diff_notes, ->{ where(type: ['Note', 'DiscussionNote', nil]) } + scope :diff_notes, -> { where(type: %w(LegacyDiffNote DiffNote)) } + scope :new_diff_notes, -> { where(type: 'DiffNote') } + scope :non_diff_notes, -> { where(type: ['Note', 'DiscussionNote', nil]) } scope :with_associations, -> do # FYI noteable cannot be loaded for LegacyDiffNote for commits diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index 9b1cac64c44..245f8dddcf9 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -66,6 +66,6 @@ class NotificationSetting < ActiveRecord::Base alias_method :failed_pipeline?, :failed_pipeline def event_enabled?(event) - respond_to?(event) && !!public_send(event) + respond_to?(event) && !!public_send(event) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/models/project.rb b/app/models/project.rb index e7baba2ef08..7010664e1c8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -415,7 +415,7 @@ class Project < ActiveRecord::Base union = Gitlab::SQL::Union.new([projects, namespaces]) - where("projects.id IN (#{union.to_sql})") + where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end def search_by_title(query) @@ -825,7 +825,7 @@ class Project < ActiveRecord::Base if template.nil? # If no template, we should create an instance. Ex `build_gitlab_ci_service` - public_send("build_#{service_name}_service") + public_send("build_#{service_name}_service") # rubocop:disable GitlabSecurity/PublicSend else Service.build_from_template(id, template) end @@ -1046,13 +1046,18 @@ class Project < ActiveRecord::Base end def change_head(branch) - repository.before_change_head - repository.rugged.references.create('HEAD', - "refs/heads/#{branch}", - force: true) - repository.copy_gitattributes(branch) - repository.after_change_head - reload_default_branch + if repository.branch_exists?(branch) + repository.before_change_head + repository.rugged.references.create('HEAD', + "refs/heads/#{branch}", + force: true) + repository.copy_gitattributes(branch) + repository.after_change_head + reload_default_branch + else + errors.add(:base, "Could not change HEAD: branch '#{branch}' does not exist") + false + end end def forked_from?(project) @@ -1326,7 +1331,7 @@ class Project < ActiveRecord::Base end def append_or_update_attribute(name, value) - old_values = public_send(name.to_s) + old_values = public_send(name.to_s) # rubocop:disable GitlabSecurity/PublicSend if Project.reflect_on_association(name).try(:macro) == :has_many && old_values.any? update_attribute(name, old_values + value) diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index c8fabb16dc1..fb1db0255aa 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -55,7 +55,7 @@ class ProjectFeature < ActiveRecord::Base end def access_level(feature) - public_send(ProjectFeature.access_level_attribute(feature)) + public_send(ProjectFeature.access_level_attribute(feature)) # rubocop:disable GitlabSecurity/PublicSend end def builds_enabled? @@ -80,7 +80,7 @@ class ProjectFeature < ActiveRecord::Base # which cannot be higher than repository access level def repository_children_level validator = lambda do |field| - level = public_send(field) || ProjectFeature::ENABLED + level = public_send(field) || ProjectFeature::ENABLED # rubocop:disable GitlabSecurity/PublicSend not_allowed = level > repository_access_level self.errors.add(field, "cannot have higher visibility level than repository access level") if not_allowed end diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb index aeaf63abab9..715b215d1db 100644 --- a/app/models/project_statistics.rb +++ b/app/models/project_statistics.rb @@ -14,7 +14,7 @@ class ProjectStatistics < ActiveRecord::Base def refresh!(only: nil) STATISTICS_COLUMNS.each do |column, generator| if only.blank? || only.include?(column) - public_send("update_#{column}") + public_send("update_#{column}") # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 3b5d0e00c70..049bebdbe42 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -64,6 +64,8 @@ class Repository @raw_repository ||= initialize_raw_repository end + alias_method :raw, :raw_repository + # Return absolute path to repository def path_to_repo @path_to_repo ||= File.expand_path( @@ -298,7 +300,7 @@ class Repository expire_method_caches(to_refresh) - to_refresh.each { |method| send(method) } + to_refresh.each { |method| send(method) } # rubocop:disable GitlabSecurity/PublicSend end def expire_branch_cache(branch_name = nil) @@ -763,7 +765,7 @@ class Repository index = Gitlab::Git::Index.new(raw_repository) if start_commit - index.read_tree(start_commit.raw_commit.tree) + index.read_tree(start_commit.rugged_commit.tree) parents = [start_commit.sha] else parents = [] diff --git a/app/models/user.rb b/app/models/user.rb index 5148886eed7..7935b89662b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -148,6 +148,8 @@ class User < ActiveRecord::Base uniqueness: { case_sensitive: false } validate :namespace_uniq, if: :username_changed? + validate :namespace_move_dir_allowed, if: :username_changed? + validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :unique_email, if: :email_changed? validate :owns_notification_email, if: :notification_email_changed? @@ -487,6 +489,12 @@ class User < ActiveRecord::Base end end + def namespace_move_dir_allowed + if namespace&.any_project_has_container_registry_tags? + errors.add(:username, 'cannot be changed if a personal project has container registry tags.') + end + end + def avatar_type unless avatar.image? errors.add :avatar, "only images allowed" @@ -528,7 +536,7 @@ class User < ActiveRecord::Base union = Gitlab::SQL::Union .new([groups.select(:id), authorized_projects.select(:namespace_id)]) - Group.where("namespaces.id IN (#{union.to_sql})") + Group.where("namespaces.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end # Returns a relation of groups the user has access to, including their parent @@ -719,8 +727,8 @@ class User < ActiveRecord::Base def sanitize_attrs %w[username skype linkedin twitter].each do |attr| - value = public_send(attr) - public_send("#{attr}=", Sanitize.clean(value)) if value.present? + value = public_send(attr) # rubocop:disable GitlabSecurity/PublicSend + public_send("#{attr}=", Sanitize.clean(value)) if value.present? # rubocop:disable GitlabSecurity/PublicSend end end @@ -779,7 +787,7 @@ class User < ActiveRecord::Base def with_defaults User.defaults.each do |k, v| - public_send("#{k}=", v) + public_send("#{k}=", v) # rubocop:disable GitlabSecurity/PublicSend end self @@ -825,7 +833,7 @@ class User < ActiveRecord::Base { name: name, username: username, - avatar_url: avatar_url + avatar_url: avatar_url(only_path: false) } end @@ -919,7 +927,7 @@ class User < ActiveRecord::Base def ci_authorized_runners @ci_authorized_runners ||= begin runner_ids = Ci::RunnerProject - .where("ci_runner_projects.project_id IN (#{ci_projects_union.to_sql})") + .where("ci_runner_projects.project_id IN (#{ci_projects_union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection .select(:runner_id) Ci::Runner.specific.where(id: runner_ids) end diff --git a/app/serializers/analytics_build_entity.rb b/app/serializers/analytics_build_entity.rb index ad7ad020b03..bdc22d71202 100644 --- a/app/serializers/analytics_build_entity.rb +++ b/app/serializers/analytics_build_entity.rb @@ -35,6 +35,6 @@ class AnalyticsBuildEntity < Grape::Entity private def url_to(route, build, id = nil) - public_send("#{route}_url", build.project.namespace, build.project, id || build) + public_send("#{route}_url", build.project.namespace, build.project, id || build) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/serializers/analytics_issue_entity.rb b/app/serializers/analytics_issue_entity.rb index 44c50f18613..b7d95ea020f 100644 --- a/app/serializers/analytics_issue_entity.rb +++ b/app/serializers/analytics_issue_entity.rb @@ -24,6 +24,6 @@ class AnalyticsIssueEntity < Grape::Entity private def url_to(route, id) - public_send("#{route}_url", request.project.namespace, request.project, id) + public_send("#{route}_url", request.project.namespace, request.project, id) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb index d6de43bcbcb..72e56a2c77f 100644 --- a/app/serializers/job_entity.rb +++ b/app/serializers/job_entity.rb @@ -46,6 +46,6 @@ class JobEntity < Grape::Entity end def path_to(route, build) - send("#{route}_path", build.project.namespace, build.project, build) + send("#{route}_path", build.project.namespace, build.project, build) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 9114f0ccc81..234fcbede03 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -16,6 +16,7 @@ module Issues spam_check(issue, current_user) issue.move_to_end + # current_user (defined in BaseService) is not available within run_after_commit block user = current_user issue.run_after_commit do NewIssueWorker.perform_async(issue.id, user.id) diff --git a/app/services/labels/transfer_service.rb b/app/services/labels/transfer_service.rb index d2ece354efc..775efed48eb 100644 --- a/app/services/labels/transfer_service.rb +++ b/app/services/labels/transfer_service.rb @@ -37,7 +37,7 @@ module Labels union = Gitlab::SQL::Union.new(label_ids) - Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq + Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq # rubocop:disable GitlabSecurity/SqlInjection end def group_labels_applied_to_issues diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 7d539fa49e6..fa0c0b7175c 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -17,6 +17,7 @@ module MergeRequests end def before_create(merge_request) + # current_user (defined in BaseService) is not available within run_after_commit block user = current_user merge_request.run_after_commit do NewMergeRequestWorker.perform_async(merge_request.id, user.id) diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index d81035e4eba..cf69007bc3b 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -10,7 +10,7 @@ module Projects end if changing_default_branch? - project.change_head(params[:default_branch]) + return error("Could not set the default branch") unless project.change_head(params[:default_branch]) end if project.update_attributes(params.except(:default_branch)) diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index df1dc736571..b32cfe158bb 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -74,7 +74,8 @@ = link_to "Profile", current_user, class: 'profile-link', data: { user: current_user.username } %li = link_to "Settings", profile_path - = render 'shared/user_dropdown_experimental_features' + %li + = link_to "Turn on new navigation", profile_preferences_path(anchor: "new-navigation") %li.divider %li = link_to "Sign out", destroy_user_session_path, method: :delete, class: "sign-out-link" diff --git a/app/views/layouts/header/_new.html.haml b/app/views/layouts/header/_new.html.haml index fa94925d537..2c1c23d6ea9 100644 --- a/app/views/layouts/header/_new.html.haml +++ b/app/views/layouts/header/_new.html.haml @@ -68,7 +68,8 @@ = link_to "Profile", current_user, class: 'profile-link', data: { user: current_user.username } %li = link_to "Settings", profile_path - = render 'shared/user_dropdown_experimental_features' + %li + = link_to "Turn off new navigation", profile_preferences_path(anchor: "new-navigation") %li.divider %li = link_to "Sign out", destroy_user_session_path, method: :delete, class: "sign-out-link" diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 9bd8bf91d1c..f08dcc0c242 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -18,8 +18,6 @@ = scheme.name .col-sm-12 %hr - %h3#experimental-features Experimental features - %hr .col-lg-4.profile-settings-sidebar#new-navigation %h4.prepend-top-0 New Navigation @@ -42,28 +40,6 @@ New .col-sm-12 %hr - .col-lg-4.profile-settings-sidebar#new-repository - %h4.prepend-top-0 - New Repository - %p - This setting allows you to turn on or off the new upcoming repository concept. - .col-lg-8.syntax-theme - .nav-wip - %p - The new repository is currently a work-in-progress concept and only usable on wide-screens. There are a number of improvements that we are working on in order to further refine the repository view. - %p - %a{ href: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/31890', target: 'blank' } Learn more - about the improvements that are coming soon! - = label_tag do - .preview= image_tag "old_repo.png" - %input.js-experiment-feature-toggle{ type: "radio", value: "false", name: "new_repo", checked: !show_new_repo? } - Old - = label_tag do - .preview= image_tag "new_repo.png" - %input.js-experiment-feature-toggle{ type: "radio", value: "true", name: "new_repo", checked: show_new_repo? } - New - .col-sm-12 - %hr .col-lg-4.profile-settings-sidebar %h4.prepend-top-0 Behavior diff --git a/app/views/projects/blob/_upload.html.haml b/app/views/projects/blob/_upload.html.haml index 32dbc1b3417..05b7dfe2872 100644 --- a/app/views/projects/blob/_upload.html.haml +++ b/app/views/projects/blob/_upload.html.haml @@ -19,7 +19,9 @@ = render 'shared/new_commit_form', placeholder: placeholder .form-actions - = button_tag button_title, class: 'btn btn-small btn-create btn-upload-file', id: 'submit-all' + = button_tag class: 'btn btn-create btn-upload-file', id: 'submit-all', type: 'button' do + = icon('spin spinner', class: 'js-loading-icon hidden' ) + = button_title = link_to _("Cancel"), '#', class: "btn btn-cancel", "data-dismiss" => "modal" - unless can?(current_user, :push_code, @project) diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 8c8aa4c78f5..25a5dfc2aaa 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -4,20 +4,21 @@ - diff_files = diffs.diff_files .content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed - .inline-parallel-buttons - - if !diffs_expanded? && diff_files.any? { |diff_file| diff_file.collapsed? } - = link_to 'Expand all', url_for(params.merge(expanded: 1, format: nil)), class: 'btn btn-default' - - if show_whitespace_toggle - - if current_controller?(:commit) - = commit_diff_whitespace_link(diffs.project, @commit, class: 'hidden-xs') - - elsif current_controller?('projects/merge_requests/diffs') - = diff_merge_request_whitespace_link(diffs.project, @merge_request, class: 'hidden-xs') - - elsif current_controller?(:compare) - = diff_compare_whitespace_link(diffs.project, params[:from], params[:to], class: 'hidden-xs') - .btn-group - = inline_diff_btn - = parallel_diff_btn - = render 'projects/diffs/stats', diff_files: diff_files + .files-changed-inner + .inline-parallel-buttons + - if !diffs_expanded? && diff_files.any? { |diff_file| diff_file.collapsed? } + = link_to 'Expand all', url_for(params.merge(expanded: 1, format: nil)), class: 'btn btn-default' + - if show_whitespace_toggle + - if current_controller?(:commit) + = commit_diff_whitespace_link(diffs.project, @commit, class: 'hidden-xs') + - elsif current_controller?('projects/merge_requests/diffs') + = diff_merge_request_whitespace_link(diffs.project, @merge_request, class: 'hidden-xs') + - elsif current_controller?(:compare) + = diff_compare_whitespace_link(diffs.project, params[:from], params[:to], class: 'hidden-xs') + .btn-group + = inline_diff_btn + = parallel_diff_btn + = render 'projects/diffs/stats', diff_files: diff_files - if render_overflow_warning?(diff_files) = render 'projects/diffs/warning', diff_files: diffs diff --git a/app/views/projects/graphs/charts.html.haml b/app/views/projects/graphs/charts.html.haml index 228c8c84792..9f5a1239a82 100644 --- a/app/views/projects/graphs/charts.html.haml +++ b/app/views/projects/graphs/charts.html.haml @@ -78,8 +78,8 @@ %script#projectChartData{ type: "application/json" } - projectChartData = {}; - - projectChartData['hour'] = { 'keys' => @commits_per_time.keys, 'values' => @commits_per_time.values } - - projectChartData['weekDays'] = { 'keys' => @commits_per_week_days.keys, 'values' => @commits_per_week_days.values } - - projectChartData['month'] = { 'keys' => @commits_per_month.keys, 'values' => @commits_per_month.values } + - projectChartData['hour'] = @commits_per_time + - projectChartData['weekDays'] = @commits_per_week_days + - projectChartData['month'] = @commits_per_month - projectChartData['languages'] = @languages = projectChartData.to_json.html_safe diff --git a/app/views/shared/_user_dropdown_experimental_features.html.haml b/app/views/shared/_user_dropdown_experimental_features.html.haml deleted file mode 100644 index 8e71407b748..00000000000 --- a/app/views/shared/_user_dropdown_experimental_features.html.haml +++ /dev/null @@ -1 +0,0 @@ -%li= link_to 'Experimental features', profile_preferences_path(anchor: 'experimental-features') diff --git a/app/workers/concerns/new_issuable.rb b/app/workers/concerns/new_issuable.rb index 3fd472bf0c1..eb0d6c9c36c 100644 --- a/app/workers/concerns/new_issuable.rb +++ b/app/workers/concerns/new_issuable.rb @@ -1,20 +1,23 @@ module NewIssuable attr_reader :issuable, :user - def ensure_objects_found(issuable_id, user_id) - @issuable = issuable_class.find_by(id: issuable_id) - unless @issuable - log_error(issuable_class, issuable_id) - return false - end + def objects_found?(issuable_id, user_id) + set_user(user_id) + set_issuable(issuable_id) + + user && issuable + end + def set_user(user_id) @user = User.find_by(id: user_id) - unless @user - log_error(User, user_id) - return false - end - true + log_error(User, user_id) unless @user + end + + def set_issuable(issuable_id) + @issuable = issuable_class.find_by(id: issuable_id) + + log_error(issuable_class, issuable_id) unless @issuable end def log_error(record_class, record_id) diff --git a/app/workers/new_issue_worker.rb b/app/workers/new_issue_worker.rb index 19a778ad522..d9a8e892e90 100644 --- a/app/workers/new_issue_worker.rb +++ b/app/workers/new_issue_worker.rb @@ -4,7 +4,7 @@ class NewIssueWorker include NewIssuable def perform(issue_id, user_id) - return unless ensure_objects_found(issue_id, user_id) + return unless objects_found?(issue_id, user_id) EventCreateService.new.open_issue(issuable, user) NotificationService.new.new_issue(issuable, user) diff --git a/app/workers/new_merge_request_worker.rb b/app/workers/new_merge_request_worker.rb index 3c8a68016ff..1910c490159 100644 --- a/app/workers/new_merge_request_worker.rb +++ b/app/workers/new_merge_request_worker.rb @@ -4,7 +4,7 @@ class NewMergeRequestWorker include NewIssuable def perform(merge_request_id, user_id) - return unless ensure_objects_found(merge_request_id, user_id) + return unless objects_found?(merge_request_id, user_id) EventCreateService.new.open_mr(issuable, user) NotificationService.new.new_merge_request(issuable, user) diff --git a/app/workers/pages_worker.rb b/app/workers/pages_worker.rb index 4eeb9666bb0..64788da7299 100644 --- a/app/workers/pages_worker.rb +++ b/app/workers/pages_worker.rb @@ -4,7 +4,7 @@ class PagesWorker sidekiq_options queue: :pages, retry: false def perform(action, *arg) - send(action, *arg) + send(action, *arg) # rubocop:disable GitlabSecurity/PublicSend end def deploy(build_id) diff --git a/bin/rspec-stackprof b/bin/rspec-stackprof index df79feb201d..810863ea4a0 100755 --- a/bin/rspec-stackprof +++ b/bin/rspec-stackprof @@ -1,5 +1,6 @@ #!/usr/bin/env ruby +require 'bundler/setup' require 'stackprof' $:.unshift 'spec' require 'rails_helper' @@ -13,4 +14,4 @@ StackProf.run(mode: :wall, out: output_file, interval: interval) do RSpec::Core::Runner.run(ARGV, $stderr, $stdout) end -system("stackprof #{output_file} --text --limit #{limit}") +system("bundle exec stackprof #{output_file} --text --limit #{limit}") diff --git a/changelogs/unreleased/13265-project_events_noteable_iid.yml b/changelogs/unreleased/13265-project_events_noteable_iid.yml new file mode 100644 index 00000000000..54d538bb548 --- /dev/null +++ b/changelogs/unreleased/13265-project_events_noteable_iid.yml @@ -0,0 +1,4 @@ +--- +title: Expose noteable_iid in Note +merge_request: 13265 +author: sue445 diff --git a/changelogs/unreleased/34339-user_avatar-url-in-push-event-webhook-json-payload-is-relative-should-be-absolute.yml b/changelogs/unreleased/34339-user_avatar-url-in-push-event-webhook-json-payload-is-relative-should-be-absolute.yml new file mode 100644 index 00000000000..13f28da8577 --- /dev/null +++ b/changelogs/unreleased/34339-user_avatar-url-in-push-event-webhook-json-payload-is-relative-should-be-absolute.yml @@ -0,0 +1,4 @@ +--- +title: Use full path of user's avatar in webhooks +merge_request: 13401 +author: Vitaliy @blackst0ne Klachkov diff --git a/changelogs/unreleased/35052-please-select-a-file-when-attempting-to-upload-or-replace-from-the-ui.yml b/changelogs/unreleased/35052-please-select-a-file-when-attempting-to-upload-or-replace-from-the-ui.yml new file mode 100644 index 00000000000..5925da14f89 --- /dev/null +++ b/changelogs/unreleased/35052-please-select-a-file-when-attempting-to-upload-or-replace-from-the-ui.yml @@ -0,0 +1,4 @@ +--- +title: improve file upload/replace experience +merge_request: +author: diff --git a/changelogs/unreleased/35098-raise-encoding-confidence-threshold.yml b/changelogs/unreleased/35098-raise-encoding-confidence-threshold.yml new file mode 100644 index 00000000000..3cdb3011f5b --- /dev/null +++ b/changelogs/unreleased/35098-raise-encoding-confidence-threshold.yml @@ -0,0 +1,4 @@ +--- +title: Raise guessed encoding confidence threshold to 50 +merge_request: 12990 +author: diff --git a/changelogs/unreleased/35136-barchart-not-display-label-at-0-hour.yml b/changelogs/unreleased/35136-barchart-not-display-label-at-0-hour.yml new file mode 100644 index 00000000000..ea8f31cca9d --- /dev/null +++ b/changelogs/unreleased/35136-barchart-not-display-label-at-0-hour.yml @@ -0,0 +1,4 @@ +--- +title: Fix bar chart does not display label at 0 hour +merge_request: 35136 +author: Jason Dai diff --git a/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml b/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml new file mode 100644 index 00000000000..04791e09b84 --- /dev/null +++ b/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml @@ -0,0 +1,4 @@ +--- +title: Add checks for branch existence before changing HEAD +merge_request: 13359 +author: Vitaliy @blackst0ne Klachkov diff --git a/changelogs/unreleased/36119-issuable-workers.yml b/changelogs/unreleased/36119-issuable-workers.yml new file mode 100644 index 00000000000..beb01ae5b1a --- /dev/null +++ b/changelogs/unreleased/36119-issuable-workers.yml @@ -0,0 +1,4 @@ +--- +title: Simplify checking if objects exist code in new issaubles workers +merge_request: +author: diff --git a/changelogs/unreleased/mk-validate-username-change-with-container-registry-tags.yml b/changelogs/unreleased/mk-validate-username-change-with-container-registry-tags.yml new file mode 100644 index 00000000000..425d5231e14 --- /dev/null +++ b/changelogs/unreleased/mk-validate-username-change-with-container-registry-tags.yml @@ -0,0 +1,4 @@ +--- +title: Add missing validation error for username change with container registry tags +merge_request: 13356 +author: diff --git a/changelogs/unreleased/rc-fix-commits-api.yml b/changelogs/unreleased/rc-fix-commits-api.yml new file mode 100644 index 00000000000..215429eaf6b --- /dev/null +++ b/changelogs/unreleased/rc-fix-commits-api.yml @@ -0,0 +1,5 @@ +--- +title: Fix the /projects/:id/repository/commits endpoint to handle dots in the ref + name when the project full path contains a `/` +merge_request: 13370 +author: diff --git a/config/application.rb b/config/application.rb index 47887bf8596..f69dab4de39 100644 --- a/config/application.rb +++ b/config/application.rb @@ -176,7 +176,7 @@ module Gitlab next unless name.include?('namespace_project') define_method(name.sub('namespace_project', 'project')) do |project, *args| - send(name, project&.namespace, project, *args) + send(name, project&.namespace, project, *args) # rubocop:disable GitlabSecurity/PublicSend end end end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 2699173fc61..5c6578d3531 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -71,7 +71,7 @@ class Settings < Settingslogic # check that `current` (string or integer) is a contant in `modul`. def verify_constant(modul, current, default) - constant = modul.constants.find{ |name| modul.const_get(name) == current } + constant = modul.constants.find { |name| modul.const_get(name) == current } value = constant.nil? ? default : modul.const_get(constant) if current.is_a? String value = modul.const_get(current.upcase) rescue default diff --git a/config/initializers/active_record_locking.rb b/config/initializers/active_record_locking.rb index 9266ff0f615..150aaa2a8c2 100644 --- a/config/initializers/active_record_locking.rb +++ b/config/initializers/active_record_locking.rb @@ -18,7 +18,7 @@ module ActiveRecord lock_col = self.class.locking_column - previous_lock_value = send(lock_col).to_i + previous_lock_value = send(lock_col).to_i # rubocop:disable GitlabSecurity/PublicSend # This line is added as a patch previous_lock_value = nil if previous_lock_value == '0' || previous_lock_value == 0 @@ -48,7 +48,7 @@ module ActiveRecord # If something went wrong, revert the version. rescue Exception - send(lock_col + '=', previous_lock_value) + send(lock_col + '=', previous_lock_value) # rubocop:disable GitlabSecurity/PublicSend raise end end diff --git a/db/migrate/20161017125927_add_unique_index_to_labels.rb b/db/migrate/20161017125927_add_unique_index_to_labels.rb index b8f6a803a0a..fcdd79d3b02 100644 --- a/db/migrate/20161017125927_add_unique_index_to_labels.rb +++ b/db/migrate/20161017125927_add_unique_index_to_labels.rb @@ -10,7 +10,7 @@ class AddUniqueIndexToLabels < ActiveRecord::Migration def up select_all('SELECT title, project_id, COUNT(id) as cnt FROM labels GROUP BY project_id, title HAVING COUNT(id) > 1').each do |label| label_title = quote_string(label['title']) - duplicated_ids = select_all("SELECT id FROM labels WHERE project_id = #{label['project_id']} AND title = '#{label_title}' ORDER BY id ASC").map{ |label| label['id'] } + duplicated_ids = select_all("SELECT id FROM labels WHERE project_id = #{label['project_id']} AND title = '#{label_title}' ORDER BY id ASC").map { |label| label['id'] } label_id = duplicated_ids.first duplicated_ids.delete(label_id) diff --git a/db/post_migrate/20170502101023_cleanup_namespaceless_pending_delete_projects.rb b/db/post_migrate/20170502101023_cleanup_namespaceless_pending_delete_projects.rb index c1e64f20109..5238a2ba1b7 100644 --- a/db/post_migrate/20170502101023_cleanup_namespaceless_pending_delete_projects.rb +++ b/db/post_migrate/20170502101023_cleanup_namespaceless_pending_delete_projects.rb @@ -30,7 +30,7 @@ class CleanupNamespacelessPendingDeleteProjects < ActiveRecord::Migration private def pending_delete_batch - connection.exec_query(find_batch).map{ |row| row['id'].to_i } + connection.exec_query(find_batch).map { |row| row['id'].to_i } end BATCH_SIZE = 5000 diff --git a/doc/README.md b/doc/README.md index 8bb8e147cd1..ca4790ceda0 100644 --- a/doc/README.md +++ b/doc/README.md @@ -3,7 +3,7 @@ Welcome to [GitLab](https://about.gitlab.com/), a Git-based fully featured platform for software development! -We offer four different products for you and your company: +GitLab offers the most scalable Git-based fully integrated platform for software development, with flexible products and subscription plans: - **GitLab Community Edition (CE)** is an [opensource product](https://gitlab.com/gitlab-org/gitlab-ce/), self-hosted, free to use. Every feature available in GitLab CE is also available on GitLab Enterprise Edition (Starter and Premium) and GitLab.com. diff --git a/doc/administration/logs.md b/doc/administration/logs.md index 4b8d5c5cc87..76e071dc673 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -9,6 +9,33 @@ documentation](http://docs.gitlab.com/ee/administration/audit_events.html) System log files are typically plain text in a standard log file format. This guide talks about how to read and use these system log files. +## `production_json.log` + +This file lives in `/var/log/gitlab/gitlab-rails/production_json.log` for +Omnibus GitLab packages or in `/home/git/gitlab/log/production_json.log` for +installations from source. (When Gitlab is running in an environment +other than production, the corresponding logfile is shown here.) + +It contains a structured log for Rails controller requests received from +GitLab, thanks to [Lograge](https://github.com/roidrage/lograge/). Note that +requests from the API [are not yet logged to this +file](https://gitlab.com/gitlab-org/gitlab-ce/issues/36189). + +Each line contains a JSON line that can be ingested by Elasticsearch, Splunk, etc. For example: + +```json +{"method":"GET","path":"/gitlab/gitlab-ce/issues/1234","format":"html","controller":"Projects::IssuesController","action":"show","status":200,"duration":229.03,"view":174.07,"db":13.24,"time":"2017-08-08T20:15:54.821Z","params":{"namespace_id":"gitlab","project_id":"gitlab-ce","id":"1234"},"remote_ip":"18.245.0.1","user_id":1,"username":"admin"} +``` + +In this example, you can see this was a GET request for a specific issue. Notice each line also contains performance data: + +1. `duration`: the total time taken to retrieve the request +2. `view`: total time taken inside the Rails views +3. `db`: total time to retrieve data from the database + +In addition, the log contains the IP address from which the request originated +(`remote_ip`) as well as the user's ID (`user_id`), and username (`username`). + ## `production.log` This file lives in `/var/log/gitlab/gitlab-rails/production.log` for diff --git a/doc/api/events.md b/doc/api/events.md index 6e530317f6c..3d5170f3f1e 100644 --- a/doc/api/events.md +++ b/doc/api/events.md @@ -338,6 +338,45 @@ Example response: "web_url":"https://gitlab.example.com/ted" }, "author_username":"ted" + }, + { + "title": null, + "project_id": 1, + "action_name": "commented on", + "target_id": 1312, + "target_iid": 1312, + "target_type": "Note", + "author_id": 1, + "data": null, + "target_title": null, + "created_at": "2015-12-04T10:33:58.089Z", + "note": { + "id": 1312, + "body": "What an awesome day!", + "attachment": null, + "author": { + "name": "Dmitriy Zaporozhets", + "username": "root", + "id": 1, + "state": "active", + "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2015-12-04T10:33:56.698Z", + "system": false, + "noteable_id": 377, + "noteable_type": "Issue", + "noteable_iid": 377 + }, + "author": { + "name": "Dmitriy Zaporozhets", + "username": "root", + "id": 1, + "state": "active", + "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", + "web_url": "http://localhost:3000/root" + }, + "author_username": "root" } ] ``` diff --git a/doc/api/notes.md b/doc/api/notes.md index 388e6989df2..e627369e17b 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -35,7 +35,8 @@ Parameters: "updated_at": "2013-10-02T10:22:45Z", "system": true, "noteable_id": 377, - "noteable_type": "Issue" + "noteable_type": "Issue", + "noteable_iid": 377 }, { "id": 305, @@ -53,7 +54,8 @@ Parameters: "updated_at": "2013-10-02T09:56:03Z", "system": true, "noteable_id": 121, - "noteable_type": "Issue" + "noteable_type": "Issue", + "noteable_iid": 121 } ] ``` @@ -267,7 +269,8 @@ Parameters: "updated_at": "2013-10-02T08:57:14Z", "system": false, "noteable_id": 2, - "noteable_type": "MergeRequest" + "noteable_type": "MergeRequest", + "noteable_iid": 2 } ``` diff --git a/doc/install/installation.md b/doc/install/installation.md index 22aedb5403e..e335fc99fbf 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -296,9 +296,9 @@ sudo usermod -aG redis git ### Clone the Source # Clone GitLab repository - sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 9-4-stable gitlab + sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 9-5-stable gitlab -**Note:** You can change `9-4-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server! +**Note:** You can change `9-5-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server! ### Configure It @@ -507,15 +507,17 @@ Check if GitLab and its environment are configured correctly: sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production + +### Compile GetText PO files + + sudo -u git -H bundle exec rake gettext:pack RAILS_ENV=production + sudo -u git -H bundle exec rake gettext:po_to_json RAILS_ENV=production + ### Compile Assets sudo -u git -H yarn install --production --pure-lockfile sudo -u git -H bundle exec rake gitlab:assets:compile RAILS_ENV=production NODE_ENV=production -### Compile GetText PO files - - sudo -u git -H bundle exec rake gettext:compile RAILS_ENV=production - ### Start Your GitLab Instance sudo service gitlab start diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md index 583ec5522fd..0399ebec86a 100644 --- a/doc/system_hooks/system_hooks.md +++ b/doc/system_hooks/system_hooks.md @@ -31,7 +31,7 @@ X-Gitlab-Event: System Hook "path": "storecloud", "path_with_namespace": "jsmith/storecloud", "project_id": 74, - "project_visibility": "private", + "project_visibility": "private" } ``` @@ -48,7 +48,7 @@ X-Gitlab-Event: System Hook "path": "underscore", "path_with_namespace": "jsmith/underscore", "project_id": 73, - "project_visibility": "internal", + "project_visibility": "internal" } ``` @@ -66,7 +66,7 @@ X-Gitlab-Event: System Hook "owner_name": "John Smith", "owner_email": "johnsmith@gmail.com", "project_visibility": "internal", - "old_path_with_namespace": "jsmith/overscore", + "old_path_with_namespace": "jsmith/overscore" } ``` @@ -84,7 +84,7 @@ X-Gitlab-Event: System Hook "owner_name": "John Smith", "owner_email": "johnsmith@gmail.com", "project_visibility": "internal", - "old_path_with_namespace": "jsmith/overscore", + "old_path_with_namespace": "jsmith/overscore" } ``` @@ -101,7 +101,7 @@ X-Gitlab-Event: System Hook "path": "storecloud", "path_with_namespace": "jsmith/storecloud", "project_id": 74, - "project_visibility": "private", + "project_visibility": "private" } ``` @@ -121,7 +121,7 @@ X-Gitlab-Event: System Hook "user_name": "John Smith", "user_username": "johnsmith", "user_id": 41, - "project_visibility": "private", + "project_visibility": "private" } ``` @@ -141,7 +141,7 @@ X-Gitlab-Event: System Hook "user_name": "John Smith", "user_username": "johnsmith", "user_id": 41, - "project_visibility": "private", + "project_visibility": "private" } ``` diff --git a/doc/update/9.1-to-9.2.md b/doc/update/9.1-to-9.2.md index f38547bba1a..6f19d16ad74 100644 --- a/doc/update/9.1-to-9.2.md +++ b/doc/update/9.1-to-9.2.md @@ -100,6 +100,7 @@ cd /home/git/gitlab sudo -u git -H git fetch --all sudo -u git -H git checkout -- db/schema.rb # local changes will be restored automatically +sudo -u git -H git checkout -- locale ``` For GitLab Community Edition: @@ -236,6 +237,11 @@ sudo -u git -H bundle clean # Run database migrations sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production +# Compile GetText PO files + +sudo -u git -H bundle exec rake gettext:pack RAILS_ENV=production +sudo -u git -H bundle exec rake gettext:po_to_json RAILS_ENV=production + # Update node dependencies and recompile assets sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production diff --git a/doc/update/9.2-to-9.3.md b/doc/update/9.2-to-9.3.md index 373f43eb3bb..9415fa1fcd6 100644 --- a/doc/update/9.2-to-9.3.md +++ b/doc/update/9.2-to-9.3.md @@ -100,6 +100,7 @@ cd /home/git/gitlab sudo -u git -H git fetch --all sudo -u git -H git checkout -- db/schema.rb # local changes will be restored automatically +sudo -u git -H git checkout -- locale ``` For GitLab Community Edition: @@ -272,6 +273,10 @@ sudo -u git -H bundle clean # Run database migrations sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production +# Compile GetText PO files + +sudo -u git -H bundle exec rake gettext:compile RAILS_ENV=production + # Update node dependencies and recompile assets sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production diff --git a/doc/update/9.3-to-9.4.md b/doc/update/9.3-to-9.4.md index b167f0737aa..982385f3c24 100644 --- a/doc/update/9.3-to-9.4.md +++ b/doc/update/9.3-to-9.4.md @@ -100,6 +100,7 @@ cd /home/git/gitlab sudo -u git -H git fetch --all sudo -u git -H git checkout -- db/schema.rb # local changes will be restored automatically +sudo -u git -H git checkout -- locale ``` For GitLab Community Edition: @@ -285,6 +286,10 @@ sudo -u git -H bundle clean # Run database migrations sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production +# Compile GetText PO files + +sudo -u git -H bundle exec rake gettext:compile RAILS_ENV=production + # Update node dependencies and recompile assets sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production diff --git a/doc/update/patch_versions.md b/doc/update/patch_versions.md index ac1bcb8f241..12408123158 100644 --- a/doc/update/patch_versions.md +++ b/doc/update/patch_versions.md @@ -35,7 +35,7 @@ current version with `cat VERSION`). cd /home/git/gitlab sudo -u git -H git fetch --all -sudo -u git -H git checkout -- Gemfile.lock db/schema.rb +sudo -u git -H git checkout -- Gemfile.lock db/schema.rb locale sudo -u git -H git checkout LATEST_TAG -b LATEST_TAG ``` @@ -56,11 +56,21 @@ sudo -u git -H bundle clean # Run database migrations sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production +### 4. Compile GetText PO files + +Internationalization was added in `v9.2.0` so these commands are only +required for versions equal or major to it. + +```bash +sudo -u git -H bundle exec rake gettext:pack RAILS_ENV=production +sudo -u git -H bundle exec rake gettext:po_to_json RAILS_ENV=production +``` + # Clean up assets and cache sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile cache:clear RAILS_ENV=production NODE_ENV=production ``` -### 4. Update gitlab-workhorse to the corresponding version +### 5. Update gitlab-workhorse to the corresponding version ```bash cd /home/git/gitlab @@ -68,7 +78,7 @@ cd /home/git/gitlab sudo -u git -H bundle exec rake "gitlab:workhorse:install[/home/git/gitlab-workhorse]" RAILS_ENV=production ``` -### 5. Update gitlab-shell to the corresponding version +### 6. Update gitlab-shell to the corresponding version ```bash cd /home/git/gitlab-shell @@ -78,14 +88,14 @@ sudo -u git -H git checkout v`cat /home/git/gitlab/GITLAB_SHELL_VERSION` -b v`ca sudo -u git -H sh -c 'if [ -x bin/compile ]; then bin/compile; fi' ``` -### 6. Start application +### 7. Start application ```bash sudo service gitlab start sudo service nginx restart ``` -### 7. Check application status +### 8. Check application status Check if GitLab and its environment are configured correctly: diff --git a/lib/api/commits.rb b/lib/api/commits.rb index a1cd8cc0058..ea78737288a 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -4,13 +4,14 @@ module API class Commits < Grape::API include PaginationParams - before { authenticate! } + COMMIT_ENDPOINT_REQUIREMENTS = API::PROJECT_ENDPOINT_REQUIREMENTS.merge(sha: API::NO_SLASH_URL_PART_REGEX) + before { authorize! :download_code, user_project } params do requires :id, type: String, desc: 'The ID of a project' end - resource :projects, requirements: { id: %r{[^/]+} } do + resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do desc 'Get a project repository commits' do success Entities::RepoCommit end @@ -21,7 +22,7 @@ module API optional :path, type: String, desc: 'The file path' use :pagination end - get ":id/repository/commits" do + get ':id/repository/commits' do path = params[:path] before = params[:until] after = params[:since] @@ -60,7 +61,7 @@ module API optional :author_email, type: String, desc: 'Author email for commit' optional :author_name, type: String, desc: 'Author name for commit' end - post ":id/repository/commits" do + post ':id/repository/commits' do authorize! :push_code, user_project attrs = declared_params @@ -79,42 +80,42 @@ module API desc 'Get a specific commit of a project' do success Entities::RepoCommitDetail - failure [[404, 'Not Found']] + failure [[404, 'Commit Not Found']] end params do requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag' end - get ":id/repository/commits/:sha" do + get ':id/repository/commits/:sha', requirements: COMMIT_ENDPOINT_REQUIREMENTS do commit = user_project.commit(params[:sha]) - not_found! "Commit" unless commit + not_found! 'Commit' unless commit present commit, with: Entities::RepoCommitDetail end desc 'Get the diff for a specific commit of a project' do - failure [[404, 'Not Found']] + failure [[404, 'Commit Not Found']] end params do requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag' end - get ":id/repository/commits/:sha/diff" do + get ':id/repository/commits/:sha/diff', requirements: COMMIT_ENDPOINT_REQUIREMENTS do commit = user_project.commit(params[:sha]) - not_found! "Commit" unless commit + not_found! 'Commit' unless commit commit.raw_diffs.to_a end desc "Get a commit's comments" do success Entities::CommitNote - failure [[404, 'Not Found']] + failure [[404, 'Commit Not Found']] end params do use :pagination requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag' end - get ':id/repository/commits/:sha/comments' do + get ':id/repository/commits/:sha/comments', requirements: COMMIT_ENDPOINT_REQUIREMENTS do commit = user_project.commit(params[:sha]) not_found! 'Commit' unless commit @@ -128,10 +129,10 @@ module API success Entities::RepoCommit end params do - requires :sha, type: String, desc: 'A commit sha to be cherry picked' + requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag to be cherry picked' requires :branch, type: String, desc: 'The name of the branch' end - post ':id/repository/commits/:sha/cherry_pick' do + post ':id/repository/commits/:sha/cherry_pick', requirements: COMMIT_ENDPOINT_REQUIREMENTS do authorize! :push_code, user_project commit = user_project.commit(params[:sha]) @@ -160,7 +161,7 @@ module API success Entities::CommitNote end params do - requires :sha, type: String, regexp: /\A\h{6,40}\z/, desc: "The commit's SHA" + requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag on which to post a comment' requires :note, type: String, desc: 'The text of the comment' optional :path, type: String, desc: 'The file path' given :path do @@ -168,7 +169,7 @@ module API requires :line_type, type: String, values: %w(new old), default: 'new', desc: 'The type of the line' end end - post ':id/repository/commits/:sha/comments' do + post ':id/repository/commits/:sha/comments', requirements: COMMIT_ENDPOINT_REQUIREMENTS do commit = user_project.commit(params[:sha]) not_found! 'Commit' unless commit diff --git a/lib/api/entities.rb b/lib/api/entities.rb index bdea308ad5d..3bb1910a441 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -132,7 +132,7 @@ module API expose :lfs_enabled?, as: :lfs_enabled expose :creator_id expose :namespace, using: 'API::Entities::Namespace' - expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda{ |project, options| project.forked? } + expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } expose :import_status expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } expose :avatar_url do |user, options| @@ -454,6 +454,9 @@ module API end class Note < Grape::Entity + # Only Issue and MergeRequest have iid + NOTEABLE_TYPES_WITH_IID = %w(Issue MergeRequest).freeze + expose :id expose :note, as: :body expose :attachment_identifier, as: :attachment @@ -461,6 +464,9 @@ module API expose :created_at, :updated_at expose :system?, as: :system expose :noteable_id, :noteable_type + + # Avoid N+1 queries as much as possible + expose(:noteable_iid) { |note| note.noteable.iid if NOTEABLE_TYPES_WITH_IID.include?(note.noteable_type) } end class AwardEmoji < Grape::Entity diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb index d9cae1501f8..a50ea0b52aa 100644 --- a/lib/api/helpers/members_helpers.rb +++ b/lib/api/helpers/members_helpers.rb @@ -1,8 +1,10 @@ +# rubocop:disable GitlabSecurity/PublicSend + module API module Helpers module MembersHelpers def find_source(source_type, id) - public_send("find_#{source_type}!", id) + public_send("find_#{source_type}!", id) # rubocop:disable GitlabSecurity/PublicSend end def authorize_admin_source!(source_type, source) diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 65ff89edf65..4e4e473994b 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -139,7 +139,7 @@ module API helpers do def find_project_noteable(noteables_str, noteable_id) - public_send("find_project_#{noteables_str.singularize}", noteable_id) + public_send("find_project_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend end def noteable_read_ability_name(noteable) diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 773f667abe0..4a2e9c9cbb0 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -68,7 +68,7 @@ module API expose :lfs_enabled?, as: :lfs_enabled expose :creator_id expose :namespace, using: 'API::Entities::Namespace' - expose :forked_from_project, using: ::API::Entities::BasicProjectDetails, if: lambda{ |project, options| project.forked? } + expose :forked_from_project, using: ::API::Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } expose :avatar_url do |user, options| user.avatar_url(only_path: false) end diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index ca6d6848d41..b9a573d3542 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -198,11 +198,11 @@ module Backup end def archives_to_backup - ARCHIVES_TO_BACKUP.map{ |name| (name + ".tar.gz") unless skipped?(name) }.compact + ARCHIVES_TO_BACKUP.map { |name| (name + ".tar.gz") unless skipped?(name) }.compact end def folders_to_backup - FOLDERS_TO_BACKUP.reject{ |name| skipped?(name) } + FOLDERS_TO_BACKUP.reject { |name| skipped?(name) } end def disabled_features diff --git a/lib/ci/ansi2html.rb b/lib/ci/ansi2html.rb index 55402101e43..8354fc8d595 100644 --- a/lib/ci/ansi2html.rb +++ b/lib/ci/ansi2html.rb @@ -254,7 +254,7 @@ module Ci def state state = STATE_PARAMS.inject({}) do |h, param| - h[param] = send(param) + h[param] = send(param) # rubocop:disable GitlabSecurity/PublicSend h end Base64.urlsafe_encode64(state.to_json) @@ -266,7 +266,7 @@ module Ci return if state[:offset].to_i > stream.size STATE_PARAMS.each do |param| - send("#{param}=".to_sym, state[param]) + send("#{param}=".to_sym, state[param]) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/lib/ci/charts.rb b/lib/ci/charts.rb index 872e418c788..76a69bf8a83 100644 --- a/lib/ci/charts.rb +++ b/lib/ci/charts.rb @@ -47,7 +47,7 @@ module Ci def collect query = project.pipelines - .where("? > #{Ci::Pipeline.table_name}.created_at AND #{Ci::Pipeline.table_name}.created_at > ?", @to, @from) + .where("? > #{Ci::Pipeline.table_name}.created_at AND #{Ci::Pipeline.table_name}.created_at > ?", @to, @from) # rubocop:disable GitlabSecurity/SqlInjection totals_count = grouped_count(query) success_count = grouped_count(query.success) diff --git a/lib/constraints/project_url_constrainer.rb b/lib/constraints/project_url_constrainer.rb index 4c0aee6c48f..fd7b97d3167 100644 --- a/lib/constraints/project_url_constrainer.rb +++ b/lib/constraints/project_url_constrainer.rb @@ -6,6 +6,8 @@ class ProjectUrlConstrainer return false unless DynamicPathValidator.valid_project_path?(full_path) + # We intentionally allow SELECT(*) here so result of this query can be used + # as cache for further Project.find_by_full_path calls within request Project.find_by_full_path(full_path, follow_redirects: request.get?).present? end end diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index 1611eba31da..d671867e7c7 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -77,8 +77,8 @@ EOM def initialize(merge_request, project) @merge_request = merge_request - @our_commit = merge_request.source_branch_head.raw.raw_commit - @their_commit = merge_request.target_branch_head.raw.raw_commit + @our_commit = merge_request.source_branch_head.raw.rugged_commit + @their_commit = merge_request.target_branch_head.raw.rugged_commit @project = project end end diff --git a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb index b260822788d..2479b4a7706 100644 --- a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb @@ -54,7 +54,7 @@ module Gitlab end def serialize_commit(event, commit, query) - commit = Commit.new(Gitlab::Git::Commit.new(commit.to_hash), @project) + commit = Commit.from_hash(commit.to_hash, @project) AnalyticsCommitSerializer.new(project: @project, total_time: event['total_time']).represent(commit) end diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index 2d89ccfc354..0603141e441 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -21,7 +21,7 @@ module Gitlab def to_hash hash = {} - serialize_keys.each { |key| hash[key] = send(key) } + serialize_keys.each { |key| hash[key] = send(key) } # rubocop:disable GitlabSecurity/PublicSend hash end diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb index 72d7d4f84d1..abd401224d8 100644 --- a/lib/gitlab/ee_compat_check.rb +++ b/lib/gitlab/ee_compat_check.rb @@ -98,10 +98,11 @@ module Gitlab if status.zero? @ee_branch_found = ee_branch_prefix - else - _, status = step("Fetching origin/#{ee_branch_suffix}", %W[git fetch origin #{ee_branch_suffix}]) + return end + _, status = step("Fetching origin/#{ee_branch_suffix}", %W[git fetch origin #{ee_branch_suffix}]) + if status.zero? @ee_branch_found = ee_branch_suffix else diff --git a/lib/gitlab/encoding_helper.rb b/lib/gitlab/encoding_helper.rb index 781f9c56a42..8ddc91e341d 100644 --- a/lib/gitlab/encoding_helper.rb +++ b/lib/gitlab/encoding_helper.rb @@ -11,7 +11,7 @@ module Gitlab # obscure encoding with low confidence. # There is a lot more info with this merge request: # https://gitlab.com/gitlab-org/gitlab_git/merge_requests/77#note_4754193 - ENCODING_CONFIDENCE_THRESHOLD = 40 + ENCODING_CONFIDENCE_THRESHOLD = 50 def encode!(message) return nil unless message.respond_to? :force_encoding diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb index 8dbe25e55f6..31effdba292 100644 --- a/lib/gitlab/git/blame.rb +++ b/lib/gitlab/git/blame.rb @@ -16,7 +16,7 @@ module Gitlab def each @blames.each do |blame| yield( - Gitlab::Git::Commit.new(blame.commit), + Gitlab::Git::Commit.new(@repo, blame.commit), blame.line ) end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 600d886e818..fd4dfdb09a2 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -14,7 +14,7 @@ module Gitlab attr_accessor *SERIALIZE_KEYS # rubocop:disable Lint/AmbiguousOperator - delegate :tree, to: :raw_commit + delegate :tree, to: :rugged_commit def ==(other) return false unless other.is_a?(Gitlab::Git::Commit) @@ -50,19 +50,29 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/321 def find(repo, commit_id = "HEAD") + # Already a commit? return commit_id if commit_id.is_a?(Gitlab::Git::Commit) - return decorate(commit_id) if commit_id.is_a?(Rugged::Commit) - obj = if commit_id.is_a?(String) - repo.rev_parse_target(commit_id) - else - Gitlab::Git::Ref.dereference_object(commit_id) - end + # A rugged reference? + commit_id = Gitlab::Git::Ref.dereference_object(commit_id) + return decorate(repo, commit_id) if commit_id.is_a?(Rugged::Commit) - return nil unless obj.is_a?(Rugged::Commit) + # Some weird thing? + return nil unless commit_id.is_a?(String) - decorate(obj) - rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError, Gitlab::Git::Repository::NoRepository + commit = repo.gitaly_migrate(:find_commit) do |is_enabled| + if is_enabled + repo.gitaly_commit_client.find_commit(commit_id) + else + obj = repo.rev_parse_target(commit_id) + + obj.is_a?(Rugged::Commit) ? obj : nil + end + end + + decorate(repo, commit) if commit + rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError, + Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository nil end @@ -102,7 +112,7 @@ module Gitlab if is_enabled repo.gitaly_commit_client.between(base, head) else - repo.rugged_commits_between(base, head).map { |c| decorate(c) } + repo.rugged_commits_between(base, head).map { |c| decorate(repo, c) } end end rescue Rugged::ReferenceError @@ -169,7 +179,7 @@ module Gitlab offset = actual_options[:skip] limit = actual_options[:max_count] walker.each(offset: offset, limit: limit) do |commit| - commits.push(decorate(commit)) + commits.push(decorate(repo, commit)) end walker.reset @@ -183,27 +193,8 @@ module Gitlab Gitlab::GitalyClient::CommitService.new(repo).find_all_commits(options) end - def decorate(commit, ref = nil) - Gitlab::Git::Commit.new(commit, ref) - end - - # Returns a diff object for the changes introduced by +rugged_commit+. - # If +rugged_commit+ doesn't have a parent, then the diff is between - # this commit and an empty repo. See Repository#diff for the keys - # allowed in the +options+ hash. - def diff_from_parent(rugged_commit, options = {}) - options ||= {} - break_rewrites = options[:break_rewrites] - actual_options = Gitlab::Git::Diff.filter_diff_options(options) - - diff = if rugged_commit.parents.empty? - rugged_commit.diff(actual_options.merge(reverse: true)) - else - rugged_commit.parents[0].diff(rugged_commit, actual_options) - end - - diff.find_similar!(break_rewrites: break_rewrites) - diff + def decorate(repository, commit, ref = nil) + Gitlab::Git::Commit.new(repository, commit, ref) end # Returns the `Rugged` sorting type constant for one or more given @@ -221,7 +212,7 @@ module Gitlab end end - def initialize(raw_commit, head = nil) + def initialize(repository, raw_commit, head = nil) raise "Nil as raw commit passed" unless raw_commit case raw_commit @@ -229,12 +220,13 @@ module Gitlab init_from_hash(raw_commit) when Rugged::Commit init_from_rugged(raw_commit) - when Gitlab::GitalyClient::Commit + when Gitaly::GitCommit init_from_gitaly(raw_commit) else raise "Invalid raw commit type: #{raw_commit.class}" end + @repository = repository @head = head end @@ -269,19 +261,50 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/324 def to_diff - diff_from_parent.patch + rugged_diff_from_parent.patch end # Returns a diff object for the changes from this commit's first parent. # If there is no parent, then the diff is between this commit and an - # empty repo. See Repository#diff for keys allowed in the +options+ + # empty repo. See Repository#diff for keys allowed in the +options+ # hash. def diff_from_parent(options = {}) - Commit.diff_from_parent(raw_commit, options) + Gitlab::GitalyClient.migrate(:commit_raw_diffs) do |is_enabled| + if is_enabled + @repository.gitaly_commit_client.diff_from_parent(self, options) + else + rugged_diff_from_parent(options) + end + end + end + + def rugged_diff_from_parent(options = {}) + options ||= {} + break_rewrites = options[:break_rewrites] + actual_options = Gitlab::Git::Diff.filter_diff_options(options) + + diff = if rugged_commit.parents.empty? + rugged_commit.diff(actual_options.merge(reverse: true)) + else + rugged_commit.parents[0].diff(rugged_commit, actual_options) + end + + diff.find_similar!(break_rewrites: break_rewrites) + diff end def deltas - @deltas ||= diff_from_parent.each_delta.map { |d| Gitlab::Git::Diff.new(d) } + @deltas ||= begin + deltas = Gitlab::GitalyClient.migrate(:commit_deltas) do |is_enabled| + if is_enabled + @repository.gitaly_commit_client.commit_deltas(self) + else + rugged_diff_from_parent.each_delta + end + end + + deltas.map { |delta| Gitlab::Git::Diff.new(delta) } + end end def has_zero_stats? @@ -296,7 +319,7 @@ module Gitlab def to_hash serialize_keys.map.with_object({}) do |key, hash| - hash[key] = send(key) + hash[key] = send(key) # rubocop:disable GitlabSecurity/PublicSend end end @@ -309,14 +332,7 @@ module Gitlab end def parents - case raw_commit - when Rugged::Commit - raw_commit.parents.map { |c| Gitlab::Git::Commit.new(c) } - when Gitlab::GitalyClient::Commit - parent_ids.map { |oid| self.class.find(raw_commit.repository, oid) }.compact - else - raise NotImplementedError, "commit source doesn't support #parents" - end + parent_ids.map { |oid| self.class.find(@repository, oid) }.compact end # Get the gpg signature of this commit. @@ -334,7 +350,7 @@ module Gitlab def to_patch(options = {}) begin - raw_commit.to_mbox(options) + rugged_commit.to_mbox(options) rescue Rugged::InvalidError => ex if ex.message =~ /commit \w+ is a merge commit/i 'Patch format is not currently supported for merge commits.' @@ -382,13 +398,21 @@ module Gitlab encode! @committer_email end + def rugged_commit + @rugged_commit ||= if raw_commit.is_a?(Rugged::Commit) + raw_commit + else + @repository.rev_parse_target(id) + end + end + private def init_from_hash(hash) raw_commit = hash.symbolize_keys serialize_keys.each do |key| - send("#{key}=", raw_commit[key]) + send("#{key}=", raw_commit[key]) # rubocop:disable GitlabSecurity/PublicSend end end @@ -415,10 +439,10 @@ module Gitlab # subject from the message to make it clearer when there's one # available but not the other. @message = (commit.body.presence || commit.subject).dup - @authored_date = Time.at(commit.author.date.seconds) + @authored_date = Time.at(commit.author.date.seconds).utc @author_name = commit.author.name.dup @author_email = commit.author.email.dup - @committed_date = Time.at(commit.committer.date.seconds) + @committed_date = Time.at(commit.committer.date.seconds).utc @committer_name = commit.committer.name.dup @committer_email = commit.committer.email.dup @parent_ids = commit.parent_ids diff --git a/lib/gitlab/git/commit_stats.rb b/lib/gitlab/git/commit_stats.rb index 57c29ad112c..00acb4763e9 100644 --- a/lib/gitlab/git/commit_stats.rb +++ b/lib/gitlab/git/commit_stats.rb @@ -16,7 +16,7 @@ module Gitlab @deletions = 0 @total = 0 - diff = commit.diff_from_parent + diff = commit.rugged_diff_from_parent diff.each_patch do |p| # TODO: Use the new Rugged convenience methods when they're released diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index 9e00abefd02..ce3d65062e8 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -143,7 +143,7 @@ module Gitlab hash = {} SERIALIZE_KEYS.each do |key| - hash[key] = send(key) + hash[key] = send(key) # rubocop:disable GitlabSecurity/PublicSend end hash @@ -221,7 +221,7 @@ module Gitlab raw_diff = hash.symbolize_keys SERIALIZE_KEYS.each do |key| - send(:"#{key}=", raw_diff[key.to_sym]) + send(:"#{key}=", raw_diff[key.to_sym]) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb index 87ed9c3ea26..6a601561c2a 100644 --- a/lib/gitlab/git/diff_collection.rb +++ b/lib/gitlab/git/diff_collection.rb @@ -28,7 +28,6 @@ module Gitlab @limits = self.class.collection_limits(options) @enforce_limits = !!options.fetch(:limits, true) @expanded = !!options.fetch(:expanded, true) - @from_gitaly = options.fetch(:from_gitaly, false) @line_count = 0 @byte_count = 0 @@ -44,7 +43,7 @@ module Gitlab return if @iterator.nil? Gitlab::GitalyClient.migrate(:commit_raw_diffs) do |is_enabled| - if is_enabled && @from_gitaly + if is_enabled && @iterator.is_a?(Gitlab::GitalyClient::DiffStitcher) each_gitaly_patch(&block) else each_rugged_patch(&block) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index f246393cfbc..371f8797ff2 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -321,7 +321,7 @@ module Gitlab options[:limit] ||= 0 options[:offset] ||= 0 - raw_log(options).map { |c| Commit.decorate(c) } + raw_log(options).map { |c| Commit.decorate(self, c) } end def count_commits(options) diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index c90ef282fdd..70177cd0fec 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -100,5 +100,9 @@ module Gitlab path = Rails.root.join(SERVER_VERSION_FILE) path.read.chomp end + + def self.encode(s) + s.dup.force_encoding(Encoding::ASCII_8BIT) + end end end diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb deleted file mode 100644 index 61fe462d762..00000000000 --- a/lib/gitlab/gitaly_client/commit.rb +++ /dev/null @@ -1,14 +0,0 @@ -module Gitlab - module GitalyClient - class Commit - attr_reader :repository, :gitaly_commit - - delegate :id, :subject, :body, :author, :committer, :parent_ids, to: :gitaly_commit - - def initialize(repository, gitaly_commit) - @repository = repository - @gitaly_commit = gitaly_commit - end - end - end -end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 3f577ac8530..692d7e02eef 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -29,22 +29,21 @@ module Gitlab request = Gitaly::CommitDiffRequest.new(request_params) response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request) - Gitlab::Git::DiffCollection.new(GitalyClient::DiffStitcher.new(response), options.merge(from_gitaly: true)) + GitalyClient::DiffStitcher.new(response) end def commit_deltas(commit) request = Gitaly::CommitDeltaRequest.new(commit_diff_request_params(commit)) response = GitalyClient.call(@repository.storage, :diff_service, :commit_delta, request) - response.flat_map do |msg| - msg.deltas.map { |d| Gitlab::Git::Diff.new(d) } - end + + response.flat_map { |msg| msg.deltas } end def tree_entry(ref, path, limit = nil) request = Gitaly::TreeEntryRequest.new( repository: @gitaly_repo, revision: ref, - path: path.dup.force_encoding(Encoding::ASCII_8BIT), + path: GitalyClient.encode(path), limit: limit.to_i ) @@ -100,15 +99,14 @@ module Gitlab def last_commit_for_path(revision, path) request = Gitaly::LastCommitForPathRequest.new( repository: @gitaly_repo, - revision: revision.force_encoding(Encoding::ASCII_8BIT), - path: path.to_s.force_encoding(Encoding::ASCII_8BIT) + revision: GitalyClient.encode(revision), + path: GitalyClient.encode(path.to_s) ) gitaly_commit = GitalyClient.call(@repository.storage, :commit_service, :last_commit_for_path, request).commit return unless gitaly_commit - commit = GitalyClient::Commit.new(@repository, gitaly_commit) - Gitlab::Git::Commit.new(commit) + Gitlab::Git::Commit.new(@repository, gitaly_commit) end def between(from, to) @@ -167,10 +165,21 @@ module Gitlab response.reduce("") { |memo, msg| memo << msg.data } end + def find_commit(revision) + request = Gitaly::FindCommitRequest.new( + repository: @gitaly_repo, + revision: GitalyClient.encode(revision) + ) + + response = GitalyClient.call(@repository.storage, :commit_service, :find_commit, request) + + response.commit + end + private def commit_diff_request_params(commit, options = {}) - parent_id = commit.parents[0]&.id || EMPTY_TREE_ID + parent_id = commit.parent_ids.first || EMPTY_TREE_ID { repository: @gitaly_repo, @@ -183,8 +192,7 @@ module Gitlab def consume_commits_response(response) response.flat_map do |message| message.commits.map do |gitaly_commit| - commit = GitalyClient::Commit.new(@repository, gitaly_commit) - Gitlab::Git::Commit.new(commit) + Gitlab::Git::Commit.new(@repository, gitaly_commit) end end end diff --git a/lib/gitlab/gitaly_client/diff.rb b/lib/gitlab/gitaly_client/diff.rb index d459c9a88fb..54df6304865 100644 --- a/lib/gitlab/gitaly_client/diff.rb +++ b/lib/gitlab/gitaly_client/diff.rb @@ -7,13 +7,13 @@ module Gitlab def initialize(params) params.each do |key, val| - public_send(:"#{key}=", val) + public_send(:"#{key}=", val) # rubocop:disable GitlabSecurity/PublicSend end end def ==(other) FIELDS.all? do |field| - public_send(field) == other.public_send(field) + public_send(field) == other.public_send(field) # rubocop:disable GitlabSecurity/PublicSend end end end diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index b0f7548b7dc..919fb68b8c7 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -16,8 +16,7 @@ module Gitlab response.flat_map do |message| message.branches.map do |branch| - gitaly_commit = GitalyClient::Commit.new(@repository, branch.target) - target_commit = Gitlab::Git::Commit.decorate(gitaly_commit) + target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target) Gitlab::Git::Branch.new(@repository, branch.name, branch.target.id, target_commit) end end @@ -102,8 +101,7 @@ module Gitlab response.flat_map do |message| message.tags.map do |gitaly_tag| if gitaly_tag.target_commit.present? - commit = GitalyClient::Commit.new(@repository, gitaly_tag.target_commit) - gitaly_commit = Gitlab::Git::Commit.new(commit) + gitaly_commit = Gitlab::Git::Commit.decorate(@repository, gitaly_tag.target_commit) end Gitlab::Git::Tag.new( @@ -141,7 +139,7 @@ module Gitlab committer_email: response.commit_committer.email.dup } - Gitlab::Git::Commit.decorate(hash) + Gitlab::Git::Commit.decorate(@repository, hash) end end end diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index 79ce784f2f2..6ad97e62941 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -10,7 +10,7 @@ module Gitlab def exists? request = Gitaly::RepositoryExistsRequest.new(repository: @gitaly_repo) - GitalyClient.call(@storage, :repository_service, :exists, request).exists + GitalyClient.call(@storage, :repository_service, :repository_exists, request).exists end def garbage_collect(create_bitmap) diff --git a/lib/gitlab/gitlab_import/client.rb b/lib/gitlab/gitlab_import/client.rb index 86fb6c51765..f1007daab5d 100644 --- a/lib/gitlab/gitlab_import/client.rb +++ b/lib/gitlab/gitlab_import/client.rb @@ -71,7 +71,7 @@ module Gitlab end def config - Gitlab.config.omniauth.providers.find{|provider| provider.name == "gitlab"} + Gitlab.config.omniauth.providers.find {|provider| provider.name == "gitlab"} end def gitlab_options diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index f5b757ace77..bc836dcc08d 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -45,7 +45,7 @@ module Gitlab end def all - REFERABLES.each { |referable| send(referable.to_s.pluralize) } + REFERABLES.each { |referable| send(referable.to_s.pluralize) } # rubocop:disable GitlabSecurity/PublicSend @references.values.flatten end diff --git a/lib/haml_lint/inline_javascript.rb b/lib/haml_lint/inline_javascript.rb index f3ddcbb9c95..05668c69006 100644 --- a/lib/haml_lint/inline_javascript.rb +++ b/lib/haml_lint/inline_javascript.rb @@ -1,14 +1,16 @@ -require 'haml_lint/haml_visitor' -require 'haml_lint/linter' -require 'haml_lint/linter_registry' +unless Rails.env.production? + require 'haml_lint/haml_visitor' + require 'haml_lint/linter' + require 'haml_lint/linter_registry' -module HamlLint - class Linter::InlineJavaScript < Linter - include LinterRegistry + module HamlLint + class Linter::InlineJavaScript < Linter + include LinterRegistry - def visit_filter(node) - return unless node.filter_type == 'javascript' - record_lint(node, 'Inline JavaScript is discouraged (https://docs.gitlab.com/ee/development/gotchas.html#do-not-use-inline-javascript-in-views)') + def visit_filter(node) + return unless node.filter_type == 'javascript' + record_lint(node, 'Inline JavaScript is discouraged (https://docs.gitlab.com/ee/development/gotchas.html#do-not-use-inline-javascript-in-views)') + end end end end diff --git a/lib/static_model.rb b/lib/static_model.rb index 185921d8fbe..60e2dd82e4e 100644 --- a/lib/static_model.rb +++ b/lib/static_model.rb @@ -18,7 +18,7 @@ module StaticModel # # Pass it along if we respond to it. def [](key) - send(key) if respond_to?(key) + send(key) if respond_to?(key) # rubocop:disable GitlabSecurity/PublicSend end def to_param diff --git a/lib/support/nginx/gitlab-pages b/lib/support/nginx/gitlab-pages index d9746c5c1aa..875c8bcbf3c 100644 --- a/lib/support/nginx/gitlab-pages +++ b/lib/support/nginx/gitlab-pages @@ -18,8 +18,11 @@ server { proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; + + proxy_cache off; + # The same address as passed to GitLab Pages: `-listen-proxy` - proxy_pass http://localhost:8090/; + proxy_pass http://localhost:8090/; } # Define custom error pages diff --git a/lib/support/nginx/gitlab-pages-ssl b/lib/support/nginx/gitlab-pages-ssl index a1ccf266835..62ed482e2bf 100644 --- a/lib/support/nginx/gitlab-pages-ssl +++ b/lib/support/nginx/gitlab-pages-ssl @@ -67,8 +67,11 @@ server { proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; + + proxy_cache off; + # The same address as passed to GitLab Pages: `-listen-proxy` - proxy_pass http://localhost:8090/; + proxy_pass http://localhost:8090/; } # Define custom error pages diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index dbb3b827b9a..1bd36bbe20a 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -41,8 +41,6 @@ namespace :gitlab do end namespace :gitlab_shell do - include SystemCheck::Helpers - desc "GitLab | Check the configuration of GitLab Shell" task check: :environment do warn_user_is_not_gitlab @@ -249,8 +247,6 @@ namespace :gitlab do end namespace :sidekiq do - include SystemCheck::Helpers - desc "GitLab | Check the configuration of Sidekiq" task check: :environment do warn_user_is_not_gitlab @@ -309,8 +305,6 @@ namespace :gitlab do end namespace :incoming_email do - include SystemCheck::Helpers - desc "GitLab | Check the configuration of Reply by email" task check: :environment do warn_user_is_not_gitlab @@ -444,8 +438,6 @@ namespace :gitlab do end namespace :ldap do - include SystemCheck::Helpers - task :check, [:limit] => :environment do |_, args| # Only show up to 100 results because LDAP directories can be very big. # This setting only affects the `rake gitlab:check` script. @@ -501,8 +493,6 @@ namespace :gitlab do end namespace :repo do - include SystemCheck::Helpers - desc "GitLab | Check the integrity of the repositories managed by GitLab" task check: :environment do Gitlab.config.repositories.storages.each do |name, repository_storage| @@ -517,8 +507,6 @@ namespace :gitlab do end namespace :user do - include SystemCheck::Helpers - desc "GitLab | Check the integrity of a specific user's repositories" task :check_repos, [:username] => :environment do |t, args| username = args[:username] || prompt("Check repository integrity for fsername? ".color(:blue)) diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index aaf00bd703a..1f504485e4c 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -21,7 +21,7 @@ namespace :gitlab do create_gitaly_configuration # In CI we run scripts/gitaly-test-build instead of this command unless ENV['CI'].present? - Bundler.with_original_env { run_command!(%w[/usr/bin/env -u RUBYOPT] + [command]) } + Bundler.with_original_env { run_command!(%w[/usr/bin/env -u RUBYOPT -u BUNDLE_GEMFILE] + [command]) } end end end diff --git a/lib/tasks/gitlab/helpers.rake b/lib/tasks/gitlab/helpers.rake index dd2d5861481..b0a24790c4a 100644 --- a/lib/tasks/gitlab/helpers.rake +++ b/lib/tasks/gitlab/helpers.rake @@ -4,5 +4,5 @@ require 'tasks/gitlab/task_helpers' StateMachines::Machine.ignore_method_conflicts = true if ENV['CRON'] namespace :gitlab do - include Gitlab::TaskHelpers + extend SystemCheck::Helpers end diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index 28b2d86eed2..d85b810ac66 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -5,6 +5,8 @@ module Gitlab TaskAbortedByUserError = Class.new(StandardError) module TaskHelpers + extend self + # Ask if the user wants to continue # # Returns "yes" the user chose to continue diff --git a/locale/ko/gitlab.po b/locale/ko/gitlab.po index 97a844ada7f..0a6fbac0880 100644 --- a/locale/ko/gitlab.po +++ b/locale/ko/gitlab.po @@ -1,4 +1,7 @@ -# chang-ho,cha <changho.cha@gmail.com>, 2017. #zanata +# Korean translations for gitlab package. +# Copyright (C) 2017 THE PACKAGE'S COPYRIGHT HOLDER +# This file is distributed under the same license as the gitlab package. +# FIRST AUTHOR <EMAIL@ADDRESS>, 2017. # Huang Tao <htve@outlook.com>, 2017. #zanata msgid "" msgstr "" @@ -8,12 +11,12 @@ msgstr "" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" -"PO-Revision-Date: 2017-08-06 09:40-0400\n" +"PO-Revision-Date: 2017-08-08 08:32-0400\n" "Last-Translator: chang-ho,cha <changho.cha@gmail.com>\n" "Language-Team: Korean (https://translate.zanata.org/project/view/GitLab)\n" "Language: ko\n" +"Plural-Forms: nplurals=1; plural=0;\n" "X-Generator: Zanata 3.9.6\n" -"Plural-Forms: nplurals=1; plural=0\n" msgid "%d commit" msgid_plural "%d commits" @@ -25,7 +28,7 @@ msgid_plural "" msgstr[0] "%s 추가 커밋은 성능 이슈를 방지하기 위해 생략되었습니다." msgid "%{commit_author_link} committed %{commit_timeago}" -msgstr "%{commit_timeago} 에 %{commit_author_link} 이(가) 커밋하였습니다. " +msgstr "%{commit_timeago} 에 %{commit_author_link} 님이 커밋하였습니다. " msgid "1 pipeline" msgid_plural "%d pipelines" @@ -791,7 +794,7 @@ msgid "Select target branch" msgstr "대상 브랜치 선택" msgid "Set a password on your account to pull or push via %{protocol}." -msgstr "%{protocol}을(를) 통해 Pull 하거나 Push하려면 계정에 패스워드를 설정하십시오." +msgstr "%{protocol} 프로토콜을 통해 Pull 하거나 Push하려면 계정에 패스워드를 설정하십시오." msgid "Set up CI" msgstr "CI 설정" @@ -1122,7 +1125,7 @@ msgid "" "You are going to remove %{group_name}.\n" "Removed groups CANNOT be restored!\n" "Are you ABSOLUTELY sure?" -msgstr "%{group_name}을(를) 제거하려고합니다.\n" +msgstr "%{group_name} 그룹을 제거하려고합니다.\n" "\"정말로\" 확실합니까?" msgid "" @@ -1130,7 +1133,7 @@ msgid "" "Removed project CANNOT be restored!\n" "Are you ABSOLUTELY sure?" msgstr "" -"%{project_name_with_namespace}을(를) 삭제하려고합니다.\n" +"%{project_name_with_namespace} 프로젝트를 삭제하려고합니다.\n" "삭제된 프로젝트를 복원 할 수 없습니다!\n" "\"정말로\" 확실합니까?" @@ -1185,7 +1188,7 @@ msgid "" "You won't be able to pull or push project code via SSH until you " "%{add_ssh_key_link} to your profile" msgstr "" -"당신의 프로필에 %{add_ssh_key_link} 을(를) 하기 전에는 SSH를 통해 프로젝트 코드를 Pull 하거나 Push 할 수 " +"당신의 프로필에 %{add_ssh_key_link} 를 하기 전에는 SSH를 통해 프로젝트 코드를 Pull 하거나 Push 할 수 " "없습니다" msgid "Your name" diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 29c449d6aa9..3d21b695af4 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -127,7 +127,7 @@ describe Admin::UsersController do describe 'POST create' do it 'creates the user' do - expect{ post :create, user: attributes_for(:user) }.to change{ User.count }.by(1) + expect { post :create, user: attributes_for(:user) }.to change { User.count }.by(1) end it 'shows only one error message for an invalid email' do diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index e478a253b3f..e00403118a0 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -24,7 +24,7 @@ describe InvitesController do describe 'GET #decline' do it 'declines user' do get :decline, id: token - expect{member.reload}.to raise_error ActiveRecord::RecordNotFound + expect {member.reload}.to raise_error ActiveRecord::RecordNotFound expect(response).to have_http_status(302) expect(flash[:notice]).to include 'You have declined the invitation to join' diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index bdee3894a13..23601c457b0 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -292,13 +292,13 @@ describe Projects::IssuesController do it 'rejects an issue recognized as a spam' do expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true) - expect { update_spam_issue }.not_to change{ issue.reload.title } + expect { update_spam_issue }.not_to change { issue.reload.title } end it 'rejects an issue recognized as a spam when recaptcha disabled' do stub_application_setting(recaptcha_enabled: false) - expect { update_spam_issue }.not_to change{ issue.reload.title } + expect { update_spam_issue }.not_to change { issue.reload.title } end it 'creates a spam log' do @@ -358,7 +358,7 @@ describe Projects::IssuesController do end it 'accepts an issue after recaptcha is verified' do - expect{ update_verified_issue }.to change{ issue.reload.title }.to(spammy_title) + expect { update_verified_issue }.to change { issue.reload.title }.to(spammy_title) end it 'marks spam log as recaptcha_verified' do diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb index 974330e2bbd..41d211ed1bb 100644 --- a/spec/controllers/projects/todos_controller_spec.rb +++ b/spec/controllers/projects/todos_controller_spec.rb @@ -67,7 +67,7 @@ describe Projects::TodosController do end it "doesn't create todo" do - expect{ go }.not_to change { user.todos.count } + expect { go }.not_to change { user.todos.count } expect(response).to have_http_status(404) end end @@ -135,7 +135,7 @@ describe Projects::TodosController do end it "doesn't create todo" do - expect{ go }.not_to change { user.todos.count } + expect { go }.not_to change { user.todos.count } expect(response).to have_http_status(404) end end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 634563fc290..275181a3d64 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -15,7 +15,7 @@ describe RegistrationsController do it 'signs the user in' do allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(false) - expect { post(:create, user_params) }.not_to change{ ActionMailer::Base.deliveries.size } + expect { post(:create, user_params) }.not_to change { ActionMailer::Base.deliveries.size } expect(subject.current_user).not_to be_nil end end diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 1c494b8c7ab..225753333ee 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -138,7 +138,7 @@ describe Snippets::NotesController do end it "deletes the note" do - expect{ delete :destroy, request_params }.to change{ Note.count }.from(1).to(0) + expect { delete :destroy, request_params }.to change { Note.count }.from(1).to(0) end context 'system note' do @@ -147,7 +147,7 @@ describe Snippets::NotesController do end it "does not delete the note" do - expect{ delete :destroy, request_params }.not_to change{ Note.count } + expect { delete :destroy, request_params }.not_to change { Note.count } end end end @@ -166,7 +166,7 @@ describe Snippets::NotesController do end it "does not update the note" do - expect{ delete :destroy, request_params }.not_to change{ Note.count } + expect { delete :destroy, request_params }.not_to change { Note.count } end end end diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb index 5e1e7dc078f..b45972b7f6b 100644 --- a/spec/features/password_reset_spec.rb +++ b/spec/features/password_reset_spec.rb @@ -16,7 +16,7 @@ feature 'Password reset' do user.send_reset_password_instructions user.update_attribute(:reset_password_sent_at, 5.minutes.ago) - expect{ forgot_password(user) }.to change{ user.reset_password_sent_at } + expect { forgot_password(user) }.to change { user.reset_password_sent_at } expect(page).to have_content(I18n.t('devise.passwords.send_paranoid_instructions')) expect(current_path).to eq new_user_session_path end @@ -27,7 +27,7 @@ feature 'Password reset' do # Reload because PG handles datetime less precisely than Ruby/Rails user.reload - expect{ forgot_password(user) }.not_to change{ user.reset_password_sent_at } + expect { forgot_password(user) }.not_to change { user.reset_password_sent_at } expect(page).to have_content(I18n.t('devise.passwords.send_paranoid_instructions')) expect(current_path).to eq new_user_session_path end diff --git a/spec/features/profiles/preferences_spec.rb b/spec/features/profiles/preferences_spec.rb index 9123aa9d155..c935cdfd5c4 100644 --- a/spec/features/profiles/preferences_spec.rb +++ b/spec/features/profiles/preferences_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Profile > Preferences' do +describe 'Profile > Preferences', :js do let(:user) { create(:user) } before do @@ -8,28 +8,32 @@ describe 'Profile > Preferences' do visit profile_preferences_path end - describe 'User changes their syntax highlighting theme', js: true do + describe 'User changes their syntax highlighting theme' do it 'creates a flash message' do choose 'user_color_scheme_id_5' + wait_for_requests + expect_preferences_saved_message end it 'updates their preference' do choose 'user_color_scheme_id_5' - allowing_for_delay do - visit page.current_path - expect(page).to have_checked_field('user_color_scheme_id_5') - end + wait_for_requests + refresh + + expect(page).to have_checked_field('user_color_scheme_id_5') end end - describe 'User changes their default dashboard', js: true do + describe 'User changes their default dashboard' do it 'creates a flash message' do select 'Starred Projects', from: 'user_dashboard' click_button 'Save' + wait_for_requests + expect_preferences_saved_message end @@ -37,12 +41,12 @@ describe 'Profile > Preferences' do select 'Starred Projects', from: 'user_dashboard' click_button 'Save' - allowing_for_delay do - find('#logo').click + wait_for_requests + + find('#logo').click - expect(page).to have_content("You don't have starred projects yet") - expect(page.current_path).to eq starred_dashboard_projects_path - end + expect(page).to have_content("You don't have starred projects yet") + expect(page.current_path).to eq starred_dashboard_projects_path find('.shortcuts-activity').trigger('click') diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 7e4d53332e5..d3d7915bebf 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -167,6 +167,21 @@ feature 'Project' do end end + describe 'activity view' do + let(:user) { create(:user, project_view: 'activity') } + let(:project) { create(:project, :repository) } + + before do + project.team << [user, :master] + sign_in user + visit project_path(project) + end + + it 'loads activity', :js do + expect(page).to have_selector('.event-item') + end + end + def remove_with_confirm(button_text, confirm_with) click_button button_text fill_in 'confirm_name_input', with: confirm_with diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index ff004d85272..1124b42721f 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -104,7 +104,7 @@ feature 'Users', js: true do end def errors_on_page(page) - page.find('#error_explanation').find('ul').all('li').map{ |item| item.text }.join("\n") + page.find('#error_explanation').find('ul').all('li').map { |item| item.text }.join("\n") end def number_of_errors_on_page(page) diff --git a/spec/fixtures/api/schemas/public_api/v4/comment.json b/spec/fixtures/api/schemas/public_api/v4/comment.json new file mode 100644 index 00000000000..52cfe86aeeb --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/comment.json @@ -0,0 +1,21 @@ +{ + "type": "object", + "required" : [ + "name", + "message", + "commit", + "release" + ], + "properties" : { + "name": { "type": "string" }, + "message": { "type": ["string", "null"] }, + "commit": { "$ref": "commit/basic.json" }, + "release": { + "oneOf": [ + { "type": "null" }, + { "$ref": "release.json" } + ] + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/public_api/v4/commit/detail.json b/spec/fixtures/api/schemas/public_api/v4/commit/detail.json new file mode 100644 index 00000000000..b7b2535c204 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/commit/detail.json @@ -0,0 +1,16 @@ +{ + "type": "object", + "allOf": [ + { "$ref": "basic.json" }, + { + "required" : [ + "stats", + "status" + ], + "properties": { + "stats": { "$ref": "../commit_stats.json" }, + "status": { "type": ["string", "null"] } + } + } + ] +} diff --git a/spec/fixtures/api/schemas/public_api/v4/commit_note.json b/spec/fixtures/api/schemas/public_api/v4/commit_note.json new file mode 100644 index 00000000000..02081989271 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/commit_note.json @@ -0,0 +1,19 @@ +{ + "type": "object", + "required" : [ + "note", + "path", + "line", + "line_type", + "author", + "created_at" + ], + "properties" : { + "note": { "type": ["string", "null"] }, + "path": { "type": ["string", "null"] }, + "line": { "type": ["integer", "null"] }, + "line_type": { "type": ["string", "null"] }, + "author": { "$ref": "user/basic.json" }, + "created_at": { "type": "date" } + } +} diff --git a/spec/fixtures/api/schemas/public_api/v4/commit_notes.json b/spec/fixtures/api/schemas/public_api/v4/commit_notes.json new file mode 100644 index 00000000000..d65a7d677ea --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/commit_notes.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "commit_note.json" } +} diff --git a/spec/fixtures/api/schemas/public_api/v4/commit_stats.json b/spec/fixtures/api/schemas/public_api/v4/commit_stats.json new file mode 100644 index 00000000000..779384c62e6 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/commit_stats.json @@ -0,0 +1,14 @@ +{ + "type": "object", + "required" : [ + "additions", + "deletions", + "total" + ], + "properties" : { + "additions": { "type": "integer" }, + "deletions": { "type": "integer" }, + "total": { "type": "integer" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/public_api/v4/commits.json b/spec/fixtures/api/schemas/public_api/v4/commits.json new file mode 100644 index 00000000000..98b17a96071 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/commits.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "commit/basic.json" } +} diff --git a/spec/fixtures/api/schemas/public_api/v4/user/basic.json b/spec/fixtures/api/schemas/public_api/v4/user/basic.json new file mode 100644 index 00000000000..9f69d31971c --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/user/basic.json @@ -0,0 +1,15 @@ +{ + "type": "object", + "required": [ + "id", + "state", + "avatar_url", + "web_url" + ], + "properties": { + "id": { "type": "integer" }, + "state": { "type": "string" }, + "avatar_url": { "type": "string" }, + "web_url": { "type": "string" } + } +} diff --git a/spec/fixtures/encoding/Japanese.md b/spec/fixtures/encoding/Japanese.md new file mode 100644 index 00000000000..dd469c9f232 --- /dev/null +++ b/spec/fixtures/encoding/Japanese.md @@ -0,0 +1,42 @@ ++++ +date = "2017-05-21T13:05:07+09:00" +title = "レイヤ" +weight = 10 + ++++ + +## このチュートリアルで扱う内容 +1. Redactedにおける2D開発でのレイヤの基本的な概要 +2. スクリーン上のスプライトの順序付け方法 + +### Redactedにおける2D開発でのレイヤの基本的な概要 +2Dにおいてはz軸が存在しないため、シーン内要素の描画順を制御するためには代替となる仕組みが必要です。 +Redactedでは**レイヤ**における**zIndex**属性を制御可能にすることで、この課題を解決しています。 +**デフォルトでは、zIndexは0となりオブジェクトはレイヤに追加された順番に描画されます。** + +レイヤにはいくつかの重要な特性があります。 + +* レイヤにはレイヤ化されたオブジェクトのみを含めることができます。(**3Dモデルは絶対に追加しないでください**） +* レイヤはレイヤ化されたオブジェクトです。（したがって、レイヤには他のレイヤを含めることができます) +* レイヤ化されたオブジェクトは、最大で1つのレイヤに属すことができます。 + +レイヤを直接初期化することはできませんが、その派生クラスは初期化することが可能です。**Scene2D**と**コンテナ**は、**レイヤ**から派生する2つの主なオブジェクトです。すべての初期化（createContainer、instantiate、...）はレイヤ上で行われます。つまり、2Dで初期化されるすべてのオブジェクトは、zIndexプロパティを持つレイヤ化されたオブジェクトです。 + +**zIndexはグローバルではありません!** + +CSSとは異なり、zIndexはすべてのオブジェクトに対してグローバルではありません。zIndexプロパティは親レイヤに対してローカルです。詳細につきましては、コンテナチュートリアルで説明しています。 [TODO: Link]。 + +### スクリーン上のスプライトの順序付け方法 +これまで学んだことを生かして、画面にスプライトを表示して、zIndexの設定をしてみましょう！ + +* まず、最初に (A,B,C) スプライトを生成します。 +* スプライトAをシーンに追加します（zIndex = 0、標準色） +* スプライトBをシーン2に追加すると、**スプライトAの上に**表示されます（zIndex = 0、赤色） +* 最後にスプライトCをシーンに追加します（青色）が、スプライトのzIndexを-1に設定すると、スプライトはAとBの後側に表示されます。 + +{{< code "static/tutorials/layers.html" >}} + +### ソースコード全体 +```js +{{< snippet "static/tutorials/layers.html" >}} +``` diff --git a/spec/javascripts/blob/blob_file_dropzone_spec.js b/spec/javascripts/blob/blob_file_dropzone_spec.js new file mode 100644 index 00000000000..2c8183ff77b --- /dev/null +++ b/spec/javascripts/blob/blob_file_dropzone_spec.js @@ -0,0 +1,42 @@ +import 'dropzone'; +import BlobFileDropzone from '~/blob/blob_file_dropzone'; + +describe('BlobFileDropzone', () => { + preloadFixtures('blob/show.html.raw'); + + beforeEach(() => { + loadFixtures('blob/show.html.raw'); + const form = $('.js-upload-blob-form'); + this.blobFileDropzone = new BlobFileDropzone(form, 'POST'); + this.dropzone = $('.js-upload-blob-form .dropzone').get(0).dropzone; + this.replaceFileButton = $('#submit-all'); + }); + + describe('submit button', () => { + it('requires file', () => { + spyOn(window, 'alert'); + + this.replaceFileButton.click(); + + expect(window.alert).toHaveBeenCalled(); + }); + + it('is disabled while uploading', () => { + spyOn(window, 'alert'); + + const file = { + name: 'some-file.jpg', + type: 'jpg', + }; + const fakeEvent = jQuery.Event('drop', { + dataTransfer: { files: [file] }, + }); + + this.dropzone.listeners[0].events.drop(fakeEvent); + this.replaceFileButton.click(); + + expect(window.alert).not.toHaveBeenCalled(); + expect(this.replaceFileButton.is(':disabled')).toEqual(true); + }); + }); +}); diff --git a/spec/javascripts/fixtures/blob.rb b/spec/javascripts/fixtures/blob.rb new file mode 100644 index 00000000000..16490ad5039 --- /dev/null +++ b/spec/javascripts/fixtures/blob.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Projects::BlobController, '(JavaScript fixtures)', type: :controller do + include JavaScriptFixturesHelpers + + let(:admin) { create(:admin) } + let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} + let(:project) { create(:project, :repository, namespace: namespace, path: 'branches-project') } + + render_views + + before(:all) do + clean_frontend_fixtures('blob/') + end + + before(:each) do + sign_in(admin) + end + + it 'blob/show.html.raw' do |example| + get(:show, + namespace_id: project.namespace, + project_id: project, + id: 'add-ipython-files/files/ipython/basic.ipynb') + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end +end diff --git a/spec/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index ab74f3e00ec..ea2a4caffaf 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -1,9 +1,11 @@ /* global bp */ +import Cookies from 'js-cookie'; import { calculateTop, hideSubLevelItems, showSubLevelItems, canShowSubItems, + canShowActiveSubItems, } from '~/fly_out_nav'; describe('Fly out sidebar navigation', () => { @@ -61,7 +63,7 @@ describe('Fly out sidebar navigation', () => { }); it('does not hude subitems on mobile', () => { - breakpointSize = 'sm'; + breakpointSize = 'xs'; hideSubLevelItems(el); @@ -121,7 +123,7 @@ describe('Fly out sidebar navigation', () => { }); it('does not show sub-items on mobile', () => { - breakpointSize = 'sm'; + breakpointSize = 'xs'; showSubLevelItems(el); @@ -170,11 +172,59 @@ describe('Fly out sidebar navigation', () => { }); it('returns false if on mobile size', () => { - breakpointSize = 'sm'; + breakpointSize = 'xs'; expect( canShowSubItems(), ).toBeFalsy(); }); }); + + describe('canShowActiveSubItems', () => { + afterEach(() => { + Cookies.remove('sidebar_collapsed'); + }); + + it('returns true by default', () => { + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + + it('returns false when cookie is false & element is active', () => { + Cookies.set('sidebar_collapsed', 'false'); + el.classList.add('active'); + + expect( + canShowActiveSubItems(el), + ).toBeFalsy(); + }); + + it('returns true when cookie is false & element is active', () => { + Cookies.set('sidebar_collapsed', 'true'); + el.classList.add('active'); + + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + + it('returns true when element is active & breakpoint is sm', () => { + breakpointSize = 'sm'; + el.classList.add('active'); + + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + + it('returns true when element is active & breakpoint is md', () => { + breakpointSize = 'md'; + el.classList.add('active'); + + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + }); }); diff --git a/spec/lib/bitbucket/paginator_spec.rb b/spec/lib/bitbucket/paginator_spec.rb index 2c972da682e..bdf10a5e2a2 100644 --- a/spec/lib/bitbucket/paginator_spec.rb +++ b/spec/lib/bitbucket/paginator_spec.rb @@ -15,7 +15,7 @@ describe Bitbucket::Paginator do expect(paginator.items).to match(['item_2']) allow(paginator).to receive(:fetch_next_page).and_return(nil) - expect{ paginator.items }.to raise_error(StopIteration) + expect { paginator.items }.to raise_error(StopIteration) end end end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index ed571a2ba05..ee28387cd48 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -1323,11 +1323,11 @@ EOT describe "Error handling" do it "fails to parse YAML" do - expect{GitlabCiYamlProcessor.new("invalid: yaml: test")}.to raise_error(Psych::SyntaxError) + expect {GitlabCiYamlProcessor.new("invalid: yaml: test")}.to raise_error(Psych::SyntaxError) end it "indicates that object is invalid" do - expect{GitlabCiYamlProcessor.new("invalid_yaml")}.to raise_error(GitlabCiYamlProcessor::ValidationError) + expect {GitlabCiYamlProcessor.new("invalid_yaml")}.to raise_error(GitlabCiYamlProcessor::ValidationError) end it "returns errors if tags parameter is invalid" do diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb index 867f7d55af7..e13406d1972 100644 --- a/spec/lib/extracts_path_spec.rb +++ b/spec/lib/extracts_path_spec.rb @@ -56,7 +56,7 @@ describe ExtractsPath do context 'subclass overrides get_id' do it 'uses ref returned by get_id' do - allow_any_instance_of(self.class).to receive(:get_id){ '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e' } + allow_any_instance_of(self.class).to receive(:get_id) { '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e' } assign_ref_vars diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb index 18843cbe992..f4dfa53f050 100644 --- a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb +++ b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb @@ -170,7 +170,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do context 'when the merge request diffs are Rugged::Patch instances' do let(:commits) { merge_request_diff.commits.map(&:to_hash) } let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) } - let(:diffs) { first_commit.diff_from_parent.patches } + let(:diffs) { first_commit.rugged_diff_from_parent.patches } let(:expected_diffs) { [] } include_examples 'updated MR diff' @@ -179,7 +179,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do context 'when the merge request diffs are Rugged::Diff::Delta instances' do let(:commits) { merge_request_diff.commits.map(&:to_hash) } let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) } - let(:diffs) { first_commit.diff_from_parent.deltas } + let(:diffs) { first_commit.rugged_diff_from_parent.deltas } let(:expected_diffs) { [] } include_examples 'updated MR diff' diff --git a/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb b/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb index d7f7b26740d..ae5b31dc12d 100644 --- a/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::BitbucketImport::ProjectCreator do has_wiki?: false) end - let(:namespace){ create(:group, owner: user) } + let(:namespace) { create(:group, owner: user) } let(:token) { "asdasd12345" } let(:secret) { "sekrettt" } let(:access_params) { { bitbucket_access_token: token, bitbucket_access_token_secret: secret } } diff --git a/spec/lib/gitlab/encoding_helper_spec.rb b/spec/lib/gitlab/encoding_helper_spec.rb index 1482ef7132d..8b14b227e65 100644 --- a/spec/lib/gitlab/encoding_helper_spec.rb +++ b/spec/lib/gitlab/encoding_helper_spec.rb @@ -30,6 +30,53 @@ describe Gitlab::EncodingHelper do it 'leaves binary string as is' do expect(ext_class.encode!(binary_string)).to eq(binary_string) end + + context 'with corrupted diff' do + let(:corrupted_diff) do + with_empty_bare_repository do |repo| + content = File.read(Rails.root.join( + 'spec/fixtures/encoding/Japanese.md').to_s) + commit_a = commit(repo, 'Japanese.md', content) + commit_b = commit(repo, 'Japanese.md', + content.sub('[TODO: Link]', '[現在作業中です: Link]')) + + repo.diff(commit_a, commit_b).each_line.map(&:content).join + end + end + + let(:cleaned_diff) do + corrupted_diff.dup.force_encoding('UTF-8') + .encode!('UTF-8', invalid: :replace, replace: '') + end + + let(:encoded_diff) do + described_class.encode!(corrupted_diff.dup) + end + + it 'does not corrupt data but remove invalid characters' do + expect(encoded_diff).to eq(cleaned_diff) + end + + def commit(repo, path, content) + oid = repo.write(content, :blob) + index = repo.index + + index.read_tree(repo.head.target.tree) unless repo.empty? + + index.add(path: path, oid: oid, mode: 0100644) + user = { name: 'Test', email: 'test@example.com' } + + Rugged::Commit.create( + repo, + tree: index.write_tree(repo), + author: user, + committer: user, + message: "Update #{path}", + parents: repo.empty? ? [] : [repo.head.target].compact, + update_ref: 'HEAD' + ) + end + end end describe '#encode_utf8' do diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 26d7a364f5b..c531d4b055f 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe Gitlab::Git::Commit, seed_helper: true do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } - let(:commit) { Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID) } + let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } let(:rugged_commit) do repository.rugged.lookup(SeedRepo::Commit::ID) end @@ -24,7 +24,7 @@ describe Gitlab::Git::Commit, seed_helper: true do } @parents = [repo.head.target] - @gitlab_parents = @parents.map { |c| Gitlab::Git::Commit.decorate(c) } + @gitlab_parents = @parents.map { |c| described_class.decorate(repository, c) } @tree = @parents.first.tree sha = Rugged::Commit.create( @@ -38,7 +38,7 @@ describe Gitlab::Git::Commit, seed_helper: true do ) @raw_commit = repo.lookup(sha) - @commit = Gitlab::Git::Commit.new(@raw_commit) + @commit = described_class.new(repository, @raw_commit) end it { expect(@commit.short_id).to eq(@raw_commit.oid[0..10]) } @@ -66,6 +66,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe "Commit info from gitaly commit" do let(:id) { 'f00' } + let(:parent_ids) { %w(b45 b46) } let(:subject) { "My commit".force_encoding('ASCII-8BIT') } let(:body) { subject + "My body".force_encoding('ASCII-8BIT') } let(:committer) do @@ -88,10 +89,11 @@ describe Gitlab::Git::Commit, seed_helper: true do subject: subject, body: body, author: author, - committer: committer + committer: committer, + parent_ids: parent_ids ) end - let(:commit) { described_class.new(Gitlab::GitalyClient::Commit.new(repository, gitaly_commit)) } + let(:commit) { described_class.new(repository, gitaly_commit) } it { expect(commit.short_id).to eq(id[0..10]) } it { expect(commit.id).to eq(id) } @@ -102,6 +104,7 @@ describe Gitlab::Git::Commit, seed_helper: true do it { expect(commit.author_name).to eq(author.name) } it { expect(commit.committer_name).to eq(committer.name) } it { expect(commit.committer_email).to eq(committer.email) } + it { expect(commit.parent_ids).to eq(parent_ids) } context 'no body' do let(:body) { "".force_encoding('ASCII-8BIT') } @@ -113,45 +116,45 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'Class methods' do describe '.find' do it "should return first head commit if without params" do - expect(Gitlab::Git::Commit.last(repository).id).to eq( + expect(described_class.last(repository).id).to eq( repository.rugged.head.target.oid ) end it "should return valid commit" do - expect(Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID)).to be_valid_commit + expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_valid_commit end it "should return valid commit for tag" do - expect(Gitlab::Git::Commit.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') + expect(described_class.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') end it "should return nil for non-commit ids" do blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb") - expect(Gitlab::Git::Commit.find(repository, blob.id)).to be_nil + expect(described_class.find(repository, blob.id)).to be_nil end it "should return nil for parent of non-commit object" do blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb") - expect(Gitlab::Git::Commit.find(repository, "#{blob.id}^")).to be_nil + expect(described_class.find(repository, "#{blob.id}^")).to be_nil end it "should return nil for nonexisting ids" do - expect(Gitlab::Git::Commit.find(repository, "+123_4532530XYZ")).to be_nil + expect(described_class.find(repository, "+123_4532530XYZ")).to be_nil end context 'with broken repo' do let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH) } it 'returns nil' do - expect(Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID)).to be_nil + expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil end end end describe '.last_for_path' do context 'no path' do - subject { Gitlab::Git::Commit.last_for_path(repository, 'master') } + subject { described_class.last_for_path(repository, 'master') } describe '#id' do subject { super().id } @@ -160,7 +163,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end context 'path' do - subject { Gitlab::Git::Commit.last_for_path(repository, 'master', 'files/ruby') } + subject { described_class.last_for_path(repository, 'master', 'files/ruby') } describe '#id' do subject { super().id } @@ -169,7 +172,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end context 'ref + path' do - subject { Gitlab::Git::Commit.last_for_path(repository, SeedRepo::Commit::ID, 'encoding') } + subject { described_class.last_for_path(repository, SeedRepo::Commit::ID, 'encoding') } describe '#id' do subject { super().id } @@ -181,7 +184,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe '.where' do context 'path is empty string' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'master', path: '', @@ -199,7 +202,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'path is nil' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'master', path: nil, @@ -217,7 +220,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref is branch name' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'master', path: 'files', @@ -237,7 +240,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref is commit id' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e", path: 'files', @@ -257,7 +260,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref is tag' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'v1.0.0', path: 'files', @@ -278,7 +281,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe '.between' do subject do - commits = Gitlab::Git::Commit.between(repository, SeedRepo::Commit::PARENT_ID, SeedRepo::Commit::ID) + commits = described_class.between(repository, SeedRepo::Commit::PARENT_ID, SeedRepo::Commit::ID) commits.map { |c| c.id } end @@ -294,12 +297,12 @@ describe Gitlab::Git::Commit, seed_helper: true do it 'should return a return a collection of commits' do commits = described_class.find_all(repository) - expect(commits).to all( be_a_kind_of(Gitlab::Git::Commit) ) + expect(commits).to all( be_a_kind_of(described_class) ) end context 'max_count' do subject do - commits = Gitlab::Git::Commit.find_all( + commits = described_class.find_all( repository, max_count: 50 ) @@ -322,7 +325,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref + max_count + skip' do subject do - commits = Gitlab::Git::Commit.find_all( + commits = described_class.find_all( repository, ref: 'master', max_count: 50, @@ -374,7 +377,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '#init_from_rugged' do - let(:gitlab_commit) { Gitlab::Git::Commit.new(rugged_commit) } + let(:gitlab_commit) { described_class.new(repository, rugged_commit) } subject { gitlab_commit } describe '#id' do @@ -384,7 +387,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '#init_from_hash' do - let(:commit) { Gitlab::Git::Commit.new(sample_commit_hash) } + let(:commit) { described_class.new(repository, sample_commit_hash) } subject { commit } describe '#id' do @@ -451,7 +454,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '#ref_names' do - let(:commit) { Gitlab::Git::Commit.find(repository, 'master') } + let(:commit) { described_class.find(repository, 'master') } subject { commit.ref_names(repository) } it 'has 1 element' do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 20d6b58d6d1..a90c848432e 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -422,11 +422,11 @@ describe Gitlab::Git::Repository, seed_helper: true do it "should fail if we create an existing branch" do @repo.create_branch('duplicated_branch', 'master') - expect{@repo.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists") + expect {@repo.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists") end it "should fail if we create a branch from a non existing ref" do - expect{@repo.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge") + expect {@repo.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge") end after(:all) do @@ -505,17 +505,22 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe "#log" do - commit_with_old_name = nil - commit_with_new_name = nil - rename_commit = nil + let(:commit_with_old_name) do + Gitlab::Git::Commit.decorate(repository, @commit_with_old_name_id) + end + let(:commit_with_new_name) do + Gitlab::Git::Commit.decorate(repository, @commit_with_new_name_id) + end + let(:rename_commit) do + Gitlab::Git::Commit.decorate(repository, @rename_commit_id) + end before(:context) do # Add new commits so that there's a renamed file in the commit history repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged - - commit_with_old_name = Gitlab::Git::Commit.decorate(new_commit_edit_old_file(repo)) - rename_commit = Gitlab::Git::Commit.decorate(new_commit_move_file(repo)) - commit_with_new_name = Gitlab::Git::Commit.decorate(new_commit_edit_new_file(repo)) + @commit_with_old_name_id = new_commit_edit_old_file(repo) + @rename_commit_id = new_commit_move_file(repo) + @commit_with_new_name_id = new_commit_edit_new_file(repo) end after(:context) do @@ -754,7 +759,7 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:options) { { ref: 'master', path: ['PROCESS.md', 'README.md'] } } def commit_files(commit) - commit.diff_from_parent.deltas.flat_map do |delta| + commit.rugged_diff_from_parent.deltas.flat_map do |delta| [delta.old_file[:path], delta.new_file[:path]].uniq.compact 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 d71e0f84c65..7fe698fcb18 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::GitalyClient::CommitService do context 'when a commit does not have a parent' do it 'sends an RPC request with empty tree ref as left commit' do - initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') + initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863').raw request = Gitaly::CommitDiffRequest.new( repository: repository_message, left_commit_id: '4b825dc642cb6eb9a060e54bf8d69288fbee4904', @@ -46,18 +46,10 @@ describe Gitlab::GitalyClient::CommitService do end end - it 'returns a Gitlab::Git::DiffCollection' do + it 'returns a Gitlab::GitalyClient::DiffStitcher' do ret = client.diff_from_parent(commit) - expect(ret).to be_kind_of(Gitlab::Git::DiffCollection) - end - - it 'passes options to Gitlab::Git::DiffCollection' do - options = { max_files: 31, max_lines: 13, from_gitaly: true } - - expect(Gitlab::Git::DiffCollection).to receive(:new).with(kind_of(Enumerable), options) - - client.diff_from_parent(commit, options) + expect(ret).to be_kind_of(Gitlab::GitalyClient::DiffStitcher) end end @@ -120,4 +112,18 @@ describe Gitlab::GitalyClient::CommitService do client.tree_entries(repository, revision, path) end end + + describe '#find_commit' do + let(:revision) { '4b825dc642cb6eb9a060e54bf8d69288fbee4904' } + it 'sends an RPC request' do + request = Gitaly::FindCommitRequest.new( + repository: repository_message, revision: revision + ) + + expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:find_commit) + .with(request, kind_of(Hash)).and_return(double(commit: nil)) + + described_class.new(repository).find_commit(revision) + end + end end diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb deleted file mode 100644 index 5c9c4ed1d7c..00000000000 --- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require 'spec_helper' - -describe Gitlab::GitalyClient::RepositoryService do - set(:project) { create(:project) } - let(:storage_name) { project.repository_storage } - let(:relative_path) { project.path_with_namespace + '.git' } - let(:client) { described_class.new(project.repository) } - - describe '#exists?' do - it 'sends an exists message' do - expect_any_instance_of(Gitaly::RepositoryService::Stub) - .to receive(:exists) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) - .and_call_original - - client.exists? - end - end -end diff --git a/spec/lib/gitlab/gitlab_import/project_creator_spec.rb b/spec/lib/gitlab/gitlab_import/project_creator_spec.rb index da48d8f0670..82548c7fd31 100644 --- a/spec/lib/gitlab/gitlab_import/project_creator_spec.rb +++ b/spec/lib/gitlab/gitlab_import/project_creator_spec.rb @@ -12,7 +12,7 @@ describe Gitlab::GitlabImport::ProjectCreator do owner: { name: "john" } }.with_indifferent_access end - let(:namespace){ create(:group, owner: user) } + let(:namespace) { create(:group, owner: user) } let(:token) { "asdffg" } let(:access_params) { { gitlab_access_token: token } } diff --git a/spec/lib/gitlab/google_code_import/project_creator_spec.rb b/spec/lib/gitlab/google_code_import/project_creator_spec.rb index aad53938d52..8d5b60d50de 100644 --- a/spec/lib/gitlab/google_code_import/project_creator_spec.rb +++ b/spec/lib/gitlab/google_code_import/project_creator_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::GoogleCodeImport::ProjectCreator do "repositoryUrls" => ["https://vim.googlecode.com/git/"] ) end - let(:namespace){ create(:group, owner: user) } + let(:namespace) { create(:group, owner: user) } before do namespace.add_owner(user) diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb index 574748756bd..cd5a1b2982b 100644 --- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::ImportExport::AttributeCleaner do - let(:relation_class){ double('relation_class').as_null_object } + let(:relation_class) { double('relation_class').as_null_object } let(:unsafe_hash) do { 'id' => 101, diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb index 292ec064a67..ca2213cd112 100644 --- a/spec/lib/gitlab/ldap/config_spec.rb +++ b/spec/lib/gitlab/ldap/config_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::LDAP::Config do describe '#initialize' do it 'requires a provider' do - expect{ described_class.new }.to raise_error ArgumentError + expect { described_class.new }.to raise_error ArgumentError end it 'works' do @@ -15,7 +15,7 @@ describe Gitlab::LDAP::Config do end it 'raises an error if a unknown provider is used' do - expect{ described_class.new 'unknown' }.to raise_error(RuntimeError) + expect { described_class.new 'unknown' }.to raise_error(RuntimeError) end end diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index 175ceec44d7..5100a5a609e 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -61,12 +61,12 @@ describe Gitlab::LDAP::User do it "finds the user if already existing" do create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') - expect{ ldap_user.save }.not_to change{ User.count } + expect { ldap_user.save }.not_to change { User.count } end it "connects to existing non-ldap user if the email matches" do existing_user = create(:omniauth_user, email: 'john@example.com', provider: "twitter") - expect{ ldap_user.save }.not_to change{ User.count } + expect { ldap_user.save }.not_to change { User.count } existing_user.reload expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid' @@ -75,7 +75,7 @@ describe Gitlab::LDAP::User do it 'connects to existing ldap user if the extern_uid changes' do existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain') - expect{ ldap_user.save }.not_to change{ User.count } + expect { ldap_user.save }.not_to change { User.count } existing_user.reload expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid' @@ -85,7 +85,7 @@ describe Gitlab::LDAP::User do it 'connects to existing ldap user if the extern_uid changes and email address has upper case characters' do existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain') - expect{ ldap_user_upper_case.save }.not_to change{ User.count } + expect { ldap_user_upper_case.save }.not_to change { User.count } existing_user.reload expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid' @@ -106,7 +106,7 @@ describe Gitlab::LDAP::User do end it "creates a new user if not found" do - expect{ ldap_user.save }.to change{ User.count }.by(1) + expect { ldap_user.save }.to change { User.count }.by(1) end context 'when signup is disabled' do diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 6c84c4a0c1e..15edb820908 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -147,7 +147,7 @@ describe Gitlab::OAuth::User do end it 'throws an error' do - expect{ oauth_user.save }.to raise_error StandardError + expect { oauth_user.save }.to raise_error StandardError end end @@ -157,7 +157,7 @@ describe Gitlab::OAuth::User do end it 'throws an error' do - expect{ oauth_user.save }.to raise_error StandardError + expect { oauth_user.save }.to raise_error StandardError end end end diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb index 2827a18515e..19710029224 100644 --- a/spec/lib/gitlab/saml/user_spec.rb +++ b/spec/lib/gitlab/saml/user_spec.rb @@ -109,7 +109,7 @@ describe Gitlab::Saml::User do end it 'does not throw an error' do - expect{ saml_user.save }.not_to raise_error + expect { saml_user.save }.not_to raise_error end end @@ -119,7 +119,7 @@ describe Gitlab::Saml::User do end it 'throws an error' do - expect{ saml_user.save }.to raise_error StandardError + expect { saml_user.save }.to raise_error StandardError end end end diff --git a/spec/lib/gitlab/sql/union_spec.rb b/spec/lib/gitlab/sql/union_spec.rb index 5346881444d..baf8f6644bf 100644 --- a/spec/lib/gitlab/sql/union_spec.rb +++ b/spec/lib/gitlab/sql/union_spec.rb @@ -19,7 +19,7 @@ describe Gitlab::SQL::Union do empty_relation = User.none union = described_class.new([empty_relation, relation_1, relation_2]) - expect{User.where("users.id IN (#{union.to_sql})").to_a}.not_to raise_error + expect {User.where("users.id IN (#{union.to_sql})").to_a}.not_to raise_error expect(union.to_sql).to eq("#{to_sql(relation_1)}\nUNION\n#{to_sql(relation_2)}") end end diff --git a/spec/lib/gitlab/version_info_spec.rb b/spec/lib/gitlab/version_info_spec.rb index e7e1a92ae54..c8a1e433d59 100644 --- a/spec/lib/gitlab/version_info_spec.rb +++ b/spec/lib/gitlab/version_info_spec.rb @@ -50,8 +50,8 @@ describe 'Gitlab::VersionInfo' do context 'unknown' do it { expect(@unknown).not_to be @v0_0_1 } it { expect(@unknown).not_to be Gitlab::VersionInfo.new } - it { expect{@unknown > @v0_0_1}.to raise_error(ArgumentError) } - it { expect{@unknown < @v0_0_1}.to raise_error(ArgumentError) } + it { expect {@unknown > @v0_0_1}.to raise_error(ArgumentError) } + it { expect {@unknown < @v0_0_1}.to raise_error(ArgumentError) } end context 'parse' do diff --git a/spec/lib/json_web_token/rsa_token_spec.rb b/spec/lib/json_web_token/rsa_token_spec.rb index e7022bd06f8..d6edc964844 100644 --- a/spec/lib/json_web_token/rsa_token_spec.rb +++ b/spec/lib/json_web_token/rsa_token_spec.rb @@ -27,7 +27,7 @@ describe JSONWebToken::RSAToken do subject { JWT.decode(rsa_encoded, rsa_key) } - it { expect{subject}.not_to raise_error } + it { expect {subject}.not_to raise_error } it { expect(subject.first).to include('key' => 'value') } it do expect(subject.second).to eq( @@ -41,7 +41,7 @@ describe JSONWebToken::RSAToken do let(:new_key) { OpenSSL::PKey::RSA.generate(512) } subject { JWT.decode(rsa_encoded, new_key) } - it { expect{subject}.to raise_error(JWT::DecodeError) } + it { expect {subject}.to raise_error(JWT::DecodeError) } end end end diff --git a/spec/lib/system_check/simple_executor_spec.rb b/spec/lib/system_check/simple_executor_spec.rb index 025ea2673b4..4de5da984ba 100644 --- a/spec/lib/system_check/simple_executor_spec.rb +++ b/spec/lib/system_check/simple_executor_spec.rb @@ -240,7 +240,7 @@ describe SystemCheck::SimpleExecutor do context 'when there is an exception' do it 'rescues the exception' do - expect{ subject.run_check(BugousCheck) }.not_to raise_exception + expect { subject.run_check(BugousCheck) }.not_to raise_exception end end end diff --git a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb b/spec/migrations/migrate_process_commit_worker_jobs_spec.rb index cf2d5827306..e5793a3c0ee 100644 --- a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb +++ b/spec/migrations/migrate_process_commit_worker_jobs_spec.rb @@ -6,7 +6,7 @@ require Rails.root.join('db', 'migrate', '20161124141322_migrate_process_commit_ describe MigrateProcessCommitWorkerJobs do let(:project) { create(:project, :repository) } let(:user) { create(:user) } - let(:commit) { project.commit.raw.raw_commit } + let(:commit) { project.commit.raw.rugged_commit } describe 'Project' do describe 'find_including_path' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 08693b5da33..c18c635d811 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -33,7 +33,6 @@ describe Commit do describe '#to_reference' do let(:project) { create(:project, :repository, path: 'sample-project') } - let(:commit) { project.commit } it 'returns a String reference to the object' do expect(commit.to_reference).to eq commit.id @@ -47,7 +46,6 @@ describe Commit do describe '#reference_link_text' do let(:project) { create(:project, :repository, path: 'sample-project') } - let(:commit) { project.commit } it 'returns a String reference to the object' do expect(commit.reference_link_text).to eq commit.short_id @@ -191,7 +189,7 @@ eos it { expect(data).to be_a(Hash) } it { expect(data[:message]).to include('adds bar folder and branch-test text file to check Repository merged_to_root_ref method') } - it { expect(data[:timestamp]).to eq('2016-09-27T14:37:46+00:00') } + it { expect(data[:timestamp]).to eq('2016-09-27T14:37:46Z') } it { expect(data[:added]).to eq(["bar/branch-test.txt"]) } it { expect(data[:modified]).to eq([]) } it { expect(data[:removed]).to eq([]) } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 6fb4794ea5f..8c4a366ef8f 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -425,7 +425,7 @@ describe CommitStatus do end it "raise exception when trying to update" do - expect{ commit_status.save }.to raise_error(ActiveRecord::StaleObjectError) + expect { commit_status.save }.to raise_error(ActiveRecord::StaleObjectError) end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index c055863d298..6d825ba68d1 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -350,7 +350,7 @@ describe Issue do subject { create(:issue, project: create(:project, :repository)) } let(:backref_text) { "issue #{subject.to_reference}" } - let(:set_mentionable_text) { ->(txt){ subject.description = txt } } + let(:set_mentionable_text) { ->(txt) { subject.description = txt } } end it_behaves_like 'a Taskable' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a1a3e70a7d2..4aada17c8c0 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -681,7 +681,7 @@ describe MergeRequest do end it 'does not crash' do - expect{ subject.diverged_commits_count }.not_to raise_error + expect { subject.diverged_commits_count }.not_to raise_error end it 'returns 0' do @@ -753,7 +753,7 @@ describe MergeRequest do subject { create(:merge_request, :simple) } let(:backref_text) { "merge request #{subject.to_reference}" } - let(:set_mentionable_text) { ->(txt){ subject.description = txt } } + let(:set_mentionable_text) { ->(txt) { subject.description = txt } } end it_behaves_like 'a Taskable' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8f951605954..4a80f41cdc9 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1301,7 +1301,7 @@ describe Project do subject { project.rename_repo } - it { expect{subject}.to raise_error(StandardError) } + it { expect {subject}.to raise_error(StandardError) } end end @@ -1832,6 +1832,11 @@ describe Project do describe '#change_head' do let(:project) { create(:project, :repository) } + it 'returns error if branch does not exist' do + expect(project.change_head('unexisted-branch')).to be false + expect(project.errors.size).to eq(1) + end + it 'calls the before_change_head and after_change_head methods' do expect(project.repository).to receive(:before_change_head) expect(project.repository).to receive(:after_change_head) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index cfa77648338..62f40c12c0a 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1224,7 +1224,7 @@ describe Repository, models: true do end describe 'skip_merges option' do - subject { repository.commits(Gitlab::Git::BRANCH_REF_PREFIX + "'test'", limit: 100, skip_merges: true).map{ |k| k.id } } + subject { repository.commits(Gitlab::Git::BRANCH_REF_PREFIX + "'test'", limit: 100, skip_merges: true).map { |k| k.id } } it { is_expected.not_to include('e56497bb5f03a90a51293fc6d516788730953899') } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0103fb6040e..6c8248eeb40 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -118,6 +118,17 @@ describe User do expect(user).to validate_uniqueness_of(:username).case_insensitive end + + context 'when username is changed' do + let(:user) { build_stubbed(:user, username: 'old_path', namespace: build_stubbed(:namespace)) } + + it 'validates move_dir is allowed for the namespace' do + expect(user.namespace).to receive(:any_project_has_container_registry_tags?).and_return(true) + user.username = 'new_path' + expect(user).to be_invalid + expect(user.errors.messages[:username].first).to match('cannot be changed if a personal project has container registry tags') + end + end end it { is_expected.to validate_presence_of(:projects_limit) } diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 8b62aa268d9..3c02e6302b4 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -44,8 +44,8 @@ describe API::CommitStatuses do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(statuses_id).to contain_exactly(status3.id, status4.id, status5.id, status6.id) - json_response.sort_by!{ |status| status['id'] } - expect(json_response.map{ |status| status['allow_failure'] }).to eq([true, false, false, false]) + json_response.sort_by! { |status| status['id'] } + expect(json_response.map { |status| status['allow_failure'] }).to eq([true, false, false, false]) end end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 0dad547735d..992a6e8d76a 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -3,29 +3,27 @@ require 'mime/types' describe API::Commits do let(:user) { create(:user) } - let(:user2) { create(:user) } - let!(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } - let!(:guest) { create(:project_member, :guest, user: user2, project: project) } - let!(:note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') } - let!(:another_note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'another comment on a commit') } + let(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let(:project) { create(:project, :repository, creator: user, path: 'my.project') } + let(:branch_with_dot) { project.repository.find_branch('ends-with.json') } + let(:branch_with_slash) { project.repository.find_branch('improve/awesome') } + + let(:project_id) { project.id } + let(:current_user) { nil } before do - project.team << [user, :reporter] + project.add_master(user) end - describe "List repository commits" do - context "authorized user" do - before do - project.team << [user2, :reporter] - end - + describe 'GET /projects/:id/repository/commits' do + context 'authorized user' do it "returns project commits" do commit = project.repository.commit - get api("/projects/#{project.id}/repository/commits", user) + get api("/projects/#{project_id}/repository/commits", user) expect(response).to have_http_status(200) - expect(json_response).to be_an Array + expect(response).to match_response_schema('public_api/v4/commits') expect(json_response.first['id']).to eq(commit.id) expect(json_response.first['committer_name']).to eq(commit.committer_name) expect(json_response.first['committer_email']).to eq(commit.committer_email) @@ -34,7 +32,7 @@ describe API::Commits do it 'include correct pagination headers' do commit_count = project.repository.count_commits(ref: 'master').to_s - get api("/projects/#{project.id}/repository/commits", user) + get api("/projects/#{project_id}/repository/commits", user) expect(response).to include_pagination_headers expect(response.headers['X-Total']).to eq(commit_count) @@ -44,8 +42,9 @@ describe API::Commits do context "unauthorized user" do it "does not return project commits" do - get api("/projects/#{project.id}/repository/commits") - expect(response).to have_http_status(401) + get api("/projects/#{project_id}/repository/commits") + + expect(response).to have_http_status(404) end end @@ -54,7 +53,7 @@ describe API::Commits do commits = project.repository.commits("master") after = commits.second.created_at - get api("/projects/#{project.id}/repository/commits?since=#{after.utc.iso8601}", user) + get api("/projects/#{project_id}/repository/commits?since=#{after.utc.iso8601}", user) expect(json_response.size).to eq 2 expect(json_response.first["id"]).to eq(commits.first.id) @@ -66,7 +65,7 @@ describe API::Commits do after = commits.second.created_at commit_count = project.repository.count_commits(ref: 'master', after: after).to_s - get api("/projects/#{project.id}/repository/commits?since=#{after.utc.iso8601}", user) + get api("/projects/#{project_id}/repository/commits?since=#{after.utc.iso8601}", user) expect(response).to include_pagination_headers expect(response.headers['X-Total']).to eq(commit_count) @@ -79,7 +78,7 @@ describe API::Commits do commits = project.repository.commits("master") before = commits.second.created_at - get api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user) + get api("/projects/#{project_id}/repository/commits?until=#{before.utc.iso8601}", user) if commits.size >= 20 expect(json_response.size).to eq(20) @@ -96,7 +95,7 @@ describe API::Commits do before = commits.second.created_at commit_count = project.repository.count_commits(ref: 'master', before: before).to_s - get api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user) + get api("/projects/#{project_id}/repository/commits?until=#{before.utc.iso8601}", user) expect(response).to include_pagination_headers expect(response.headers['X-Total']).to eq(commit_count) @@ -106,7 +105,7 @@ describe API::Commits do context "invalid xmlschema date parameters" do it "returns an invalid parameter error message" do - get api("/projects/#{project.id}/repository/commits?since=invalid-date", user) + get api("/projects/#{project_id}/repository/commits?since=invalid-date", user) expect(response).to have_http_status(400) expect(json_response['error']).to eq('since is invalid') @@ -118,7 +117,7 @@ describe API::Commits do path = 'files/ruby/popen.rb' commit_count = project.repository.count_commits(ref: 'master', path: path).to_s - get api("/projects/#{project.id}/repository/commits?path=#{path}", user) + get api("/projects/#{project_id}/repository/commits?path=#{path}", user) expect(json_response.size).to eq(3) expect(json_response.first["id"]).to eq("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") @@ -130,7 +129,7 @@ describe API::Commits do path = 'files/ruby/popen.rb' commit_count = project.repository.count_commits(ref: 'master', path: path).to_s - get api("/projects/#{project.id}/repository/commits?path=#{path}", user) + get api("/projects/#{project_id}/repository/commits?path=#{path}", user) expect(response).to include_pagination_headers expect(response.headers['X-Total']).to eq(commit_count) @@ -143,7 +142,7 @@ describe API::Commits do let(:per_page) { 5 } let(:ref_name) { 'master' } let!(:request) do - get api("/projects/#{project.id}/repository/commits?page=#{page}&per_page=#{per_page}&ref_name=#{ref_name}", user) + get api("/projects/#{project_id}/repository/commits?page=#{page}&per_page=#{per_page}&ref_name=#{ref_name}", user) end it 'returns correct headers' do @@ -181,10 +180,10 @@ describe API::Commits do end describe "POST /projects/:id/repository/commits" do - let!(:url) { "/projects/#{project.id}/repository/commits" } + let!(:url) { "/projects/#{project_id}/repository/commits" } it 'returns a 403 unauthorized for user without permissions' do - post api(url, user2) + post api(url, guest) expect(response).to have_http_status(403) end @@ -227,7 +226,7 @@ describe API::Commits do it 'a new file in project repo' do post api(url, user), valid_c_params - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq(message) expect(json_response['committer_name']).to eq(user.name) expect(json_response['committer_email']).to eq(user.email) @@ -453,13 +452,17 @@ describe API::Commits do end end - describe "Get a single commit" do - context "authorized user" do - it "returns a commit by sha" do - get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) + describe 'GET /projects/:id/repository/commits/:sha' do + let(:commit) { project.repository.commit } + let(:commit_id) { commit.id } + let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}" } - expect(response).to have_http_status(200) - commit = project.repository.commit + shared_examples_for 'ref commit' do + it 'returns the ref last commit' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit/detail') expect(json_response['id']).to eq(commit.id) expect(json_response['short_id']).to eq(commit.short_id) expect(json_response['title']).to eq(commit.title) @@ -474,222 +477,539 @@ describe API::Commits do expect(json_response['stats']['additions']).to eq(commit.stats.additions) expect(json_response['stats']['deletions']).to eq(commit.stats.deletions) expect(json_response['stats']['total']).to eq(commit.stats.total) + expect(json_response['status']).to be_nil end - it "returns a 404 error if not found" do - get api("/projects/#{project.id}/repository/commits/invalid_sha", user) - expect(response).to have_http_status(404) + context 'when ref does not exist' do + let(:commit_id) { 'unknown' } + + it_behaves_like '404 response' do + let(:request) { get api(route, current_user) } + let(:message) { '404 Commit Not Found' } + end end - it "returns nil for commit without CI" do - get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) + context 'when repository is disabled' do + include_context 'disabled repository' - expect(response).to have_http_status(200) - expect(json_response['status']).to be_nil + it_behaves_like '403 response' do + let(:request) { get api(route, current_user) } + end end + end - it "returns status for CI" do - pipeline = project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha) - pipeline.update(status: 'success') + context 'when unauthenticated', 'and project is public' do + let(:project) { create(:project, :public, :repository) } - get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) + it_behaves_like 'ref commit' + end - expect(response).to have_http_status(200) - expect(json_response['status']).to eq(pipeline.status) + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { get api(route) } + let(:message) { '404 Project Not Found' } end + end - it "returns status for CI when pipeline is created" do - project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha) + context 'when authenticated', 'as a master' do + let(:current_user) { user } - get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) + it_behaves_like 'ref commit' - expect(response).to have_http_status(200) - expect(json_response['status']).to eq("created") + context 'when branch contains a dot' do + let(:commit) { project.repository.commit(branch_with_dot.name) } + let(:commit_id) { branch_with_dot.name } + + it_behaves_like 'ref commit' end - end - context "unauthorized user" do - it "does not return the selected commit" do - get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}") - expect(response).to have_http_status(401) + context 'when branch contains a slash' do + let(:commit_id) { branch_with_slash.name } + + it_behaves_like '404 response' do + let(:request) { get api(route, current_user) } + end + end + + context 'when branch contains an escaped slash' do + let(:commit) { project.repository.commit(branch_with_slash.name) } + let(:commit_id) { CGI.escape(branch_with_slash.name) } + + it_behaves_like 'ref commit' + end + + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } + + it_behaves_like 'ref commit' + + context 'when branch contains a dot' do + let(:commit) { project.repository.commit(branch_with_dot.name) } + let(:commit_id) { branch_with_dot.name } + + it_behaves_like 'ref commit' + end + end + + context 'when the ref has a pipeline' do + let!(:pipeline) { project.pipelines.create(source: :push, ref: 'master', sha: commit.sha) } + + it 'includes a "created" status' do + get api(route, current_user) + + expect(response).to have_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(json_response['status']).to eq('created') + end + + context 'when pipeline succeeds' do + before do + pipeline.update(status: 'success') + end + + it 'includes a "success" status' do + get api(route, current_user) + + expect(response).to have_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(json_response['status']).to eq('success') + end + end end end end - describe "Get the diff of a commit" do - context "authorized user" do - before do - project.team << [user2, :reporter] + describe 'GET /projects/:id/repository/commits/:sha/diff' do + let(:commit) { project.repository.commit } + let(:commit_id) { commit.id } + let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}/diff" } + + shared_examples_for 'ref diff' do + it 'returns the diff of the selected commit' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response.size).to be >= 1 + expect(json_response.first.keys).to include 'diff' end - it "returns the diff of the selected commit" do - get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/diff", user) - expect(response).to have_http_status(200) + context 'when ref does not exist' do + let(:commit_id) { 'unknown' } - expect(json_response).to be_an Array - expect(json_response.length).to be >= 1 - expect(json_response.first.keys).to include "diff" + it_behaves_like '404 response' do + let(:request) { get api(route, current_user) } + let(:message) { '404 Commit Not Found' } + end end - it "returns a 404 error if invalid commit" do - get api("/projects/#{project.id}/repository/commits/invalid_sha/diff", user) - expect(response).to have_http_status(404) + context 'when repository is disabled' do + include_context 'disabled repository' + + it_behaves_like '403 response' do + let(:request) { get api(route, current_user) } + end end end - context "unauthorized user" do - it "does not return the diff of the selected commit" do - get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/diff") - expect(response).to have_http_status(401) + context 'when unauthenticated', 'and project is public' do + let(:project) { create(:project, :public, :repository) } + + it_behaves_like 'ref diff' + end + + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { get api(route) } + let(:message) { '404 Project Not Found' } + end + end + + context 'when authenticated', 'as a master' do + let(:current_user) { user } + + it_behaves_like 'ref diff' + + context 'when branch contains a dot' do + let(:commit_id) { branch_with_dot.name } + + it_behaves_like 'ref diff' + end + + context 'when branch contains a slash' do + let(:commit_id) { branch_with_slash.name } + + it_behaves_like '404 response' do + let(:request) { get api(route, current_user) } + end + end + + context 'when branch contains an escaped slash' do + let(:commit_id) { CGI.escape(branch_with_slash.name) } + + it_behaves_like 'ref diff' + end + + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } + + it_behaves_like 'ref diff' + + context 'when branch contains a dot' do + let(:commit_id) { branch_with_dot.name } + + it_behaves_like 'ref diff' + end end end end - describe 'Get the comments of a commit' do - context 'authorized user' do - it 'returns merge_request comments' do - get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user) - expect(response).to have_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(2) - expect(json_response.first['note']).to eq('a comment on a commit') - expect(json_response.first['author']['id']).to eq(user.id) + describe 'GET /projects/:id/repository/commits/:sha/comments' do + let(:commit) { project.repository.commit } + let(:commit_id) { commit.id } + let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}/comments" } + + shared_examples_for 'ref comments' do + context 'when ref exists' do + before do + create(:note_on_commit, author: user, project: project, commit_id: commit.id, note: 'a comment on a commit') + create(:note_on_commit, author: user, project: project, commit_id: commit.id, note: 'another comment on a commit') + end + + it 'returns the diff of the selected commit' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit_notes') + expect(json_response.size).to eq(2) + expect(json_response.first['note']).to eq('a comment on a commit') + expect(json_response.first['author']['id']).to eq(user.id) + end end - it 'returns a 404 error if merge_request_id not found' do - get api("/projects/#{project.id}/repository/commits/1234ab/comments", user) - expect(response).to have_http_status(404) + context 'when ref does not exist' do + let(:commit_id) { 'unknown' } + + it_behaves_like '404 response' do + let(:request) { get api(route, current_user) } + let(:message) { '404 Commit Not Found' } + end + end + + context 'when repository is disabled' do + include_context 'disabled repository' + + it_behaves_like '403 response' do + let(:request) { get api(route, current_user) } + end + end + end + + context 'when unauthenticated', 'and project is public' do + let(:project) { create(:project, :public, :repository) } + + it_behaves_like 'ref comments' + end + + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { get api(route) } + let(:message) { '404 Project Not Found' } end end - context 'unauthorized user' do - it 'does not return the diff of the selected commit' do - get api("/projects/#{project.id}/repository/commits/1234ab/comments") - expect(response).to have_http_status(401) + context 'when authenticated', 'as a master' do + let(:current_user) { user } + + it_behaves_like 'ref comments' + + context 'when branch contains a dot' do + let(:commit) { project.repository.commit(branch_with_dot.name) } + let(:commit_id) { branch_with_dot.name } + + it_behaves_like 'ref comments' + end + + context 'when branch contains a slash' do + let(:commit) { project.repository.commit(branch_with_slash.name) } + let(:commit_id) { branch_with_slash.name } + + it_behaves_like '404 response' do + let(:request) { get api(route, current_user) } + end + end + + context 'when branch contains an escaped slash' do + let(:commit) { project.repository.commit(branch_with_slash.name) } + let(:commit_id) { CGI.escape(branch_with_slash.name) } + + it_behaves_like 'ref comments' + end + + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } + + it_behaves_like 'ref comments' + + context 'when branch contains a dot' do + let(:commit) { project.repository.commit(branch_with_dot.name) } + let(:commit_id) { branch_with_dot.name } + + it_behaves_like 'ref comments' + end end end context 'when the commit is present on two projects' do - let(:forked_project) { create(:project, :repository, creator: user2, namespace: user2.namespace) } - let!(:forked_project_note) { create(:note_on_commit, author: user2, project: forked_project, commit_id: forked_project.repository.commit.id, note: 'a comment on a commit for fork') } + let(:forked_project) { create(:project, :repository, creator: guest, namespace: guest.namespace) } + let!(:forked_project_note) { create(:note_on_commit, author: guest, project: forked_project, commit_id: forked_project.repository.commit.id, note: 'a comment on a commit for fork') } + let(:project_id) { forked_project.id } + let(:commit_id) { forked_project.repository.commit.id } it 'returns the comments for the target project' do - get api("/projects/#{forked_project.id}/repository/commits/#{forked_project.repository.commit.id}/comments", user2) + get api(route, guest) - expect(response).to have_http_status(200) - expect(json_response.length).to eq(1) + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit_notes') + expect(json_response.size).to eq(1) expect(json_response.first['note']).to eq('a comment on a commit for fork') - expect(json_response.first['author']['id']).to eq(user2.id) + expect(json_response.first['author']['id']).to eq(guest.id) end end end describe 'POST :id/repository/commits/:sha/cherry_pick' do - let(:master_pickable_commit) { project.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } + let(:commit) { project.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } + let(:commit_id) { commit.id } + let(:branch) { 'master' } + let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}/cherry_pick" } + + shared_examples_for 'ref cherry-pick' do + context 'when ref exists' do + it 'cherry-picks the ref commit' do + post api(route, current_user), branch: branch + + expect(response).to have_gitlab_http_status(201) + expect(response).to match_response_schema('public_api/v4/commit/basic') + expect(json_response['title']).to eq(commit.title) + expect(json_response['message']).to eq(commit.message) + expect(json_response['author_name']).to eq(commit.author_name) + expect(json_response['committer_name']).to eq(user.name) + end + end - context 'authorized user' do - it 'cherry picks a commit' do - post api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user), branch: 'master' + context 'when repository is disabled' do + include_context 'disabled repository' - expect(response).to have_http_status(201) - expect(json_response['title']).to eq(master_pickable_commit.title) - expect(json_response['message']).to eq(master_pickable_commit.message) - expect(json_response['author_name']).to eq(master_pickable_commit.author_name) - expect(json_response['committer_name']).to eq(user.name) + it_behaves_like '403 response' do + let(:request) { post api(route, current_user), branch: 'master' } + end end + end - it 'returns 400 if commit is already included in the target branch' do - post api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user), branch: 'markdown' + context 'when unauthenticated', 'and project is public' do + let(:project) { create(:project, :public, :repository) } - expect(response).to have_http_status(400) - expect(json_response['message']).to include('Sorry, we cannot cherry-pick this commit automatically.') + it_behaves_like '403 response' do + let(:request) { post api(route), branch: 'master' } + end + end + + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { post api(route), branch: 'master' } + let(:message) { '404 Project Not Found' } end + end - it 'returns 400 if you are not allowed to push to the target branch' do - project.team << [user2, :developer] - protected_branch = create(:protected_branch, project: project, name: 'feature') + context 'when authenticated', 'as an owner' do + let(:current_user) { user } - post api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user2), branch: protected_branch.name + it_behaves_like 'ref cherry-pick' - expect(response).to have_http_status(400) - expect(json_response['message']).to eq('You are not allowed to push into this branch') + context 'when ref does not exist' do + let(:commit_id) { 'unknown' } + + it_behaves_like '404 response' do + let(:request) { post api(route, current_user), branch: 'master' } + let(:message) { '404 Commit Not Found' } + end + end + + context 'when branch is missing' do + it_behaves_like '400 response' do + let(:request) { post api(route, current_user) } + end end - it 'returns 400 for missing parameters' do - post api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user) + context 'when branch does not exist' do + it_behaves_like '404 response' do + let(:request) { post api(route, current_user), branch: 'foo' } + let(:message) { '404 Branch Not Found' } + end + end - expect(response).to have_http_status(400) - expect(json_response['error']).to eq('branch is missing') + context 'when commit is already included in the target branch' do + it_behaves_like '400 response' do + let(:request) { post api(route, current_user), branch: 'markdown' } + end end - it 'returns 404 if commit is not found' do - post api("/projects/#{project.id}/repository/commits/abcd0123/cherry_pick", user), branch: 'master' + context 'when ref contains a dot' do + let(:commit) { project.repository.commit(branch_with_dot.name) } + let(:commit_id) { branch_with_dot.name } - expect(response).to have_http_status(404) - expect(json_response['message']).to eq('404 Commit Not Found') + it_behaves_like 'ref cherry-pick' end - it 'returns 404 if branch is not found' do - post api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user), branch: 'foo' + context 'when ref contains a slash' do + let(:commit_id) { branch_with_slash.name } - expect(response).to have_http_status(404) - expect(json_response['message']).to eq('404 Branch Not Found') + it_behaves_like '404 response' do + let(:request) { post api(route, current_user), branch: 'master' } + end end - it 'returns 400 for missing parameters' do - post api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user) + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } - expect(response).to have_http_status(400) - expect(json_response['error']).to eq('branch is missing') + it_behaves_like 'ref cherry-pick' + + context 'when ref contains a dot' do + let(:commit) { project.repository.commit(branch_with_dot.name) } + let(:commit_id) { branch_with_dot.name } + + it_behaves_like 'ref cherry-pick' + end end end - context 'unauthorized user' do - it 'does not cherry pick the commit' do - post api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick"), branch: 'master' + context 'when authenticated', 'as a developer' do + let(:current_user) { guest } + + before do + project.add_developer(guest) + end + + context 'when branch is protected' do + before do + create(:protected_branch, project: project, name: 'feature') + end + + it 'returns 400 if you are not allowed to push to the target branch' do + post api(route, current_user), branch: 'feature' - expect(response).to have_http_status(401) + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq('You are not allowed to push into this branch') + end end end end - describe 'Post comment to commit' do - context 'authorized user' do - it 'returns comment' do - post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment' - expect(response).to have_http_status(201) - expect(json_response['note']).to eq('My comment') - expect(json_response['path']).to be_nil - expect(json_response['line']).to be_nil - expect(json_response['line_type']).to be_nil + describe 'POST /projects/:id/repository/commits/:sha/comments' do + let(:commit) { project.repository.commit } + let(:commit_id) { commit.id } + let(:note) { 'My comment' } + let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}/comments" } + + shared_examples_for 'ref new comment' do + context 'when ref exists' do + it 'creates the comment' do + post api(route, current_user), note: note + + expect(response).to have_gitlab_http_status(201) + expect(response).to match_response_schema('public_api/v4/commit_note') + expect(json_response['note']).to eq('My comment') + expect(json_response['path']).to be_nil + expect(json_response['line']).to be_nil + expect(json_response['line_type']).to be_nil + end end + context 'when repository is disabled' do + include_context 'disabled repository' + + it_behaves_like '403 response' do + let(:request) { post api(route, current_user), note: 'My comment' } + end + end + end + + context 'when unauthenticated', 'and project is public' do + let(:project) { create(:project, :public, :repository) } + + it_behaves_like '400 response' do + let(:request) { post api(route), note: 'My comment' } + end + end + + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { post api(route), note: 'My comment' } + let(:message) { '404 Project Not Found' } + end + end + + context 'when authenticated', 'as an owner' do + let(:current_user) { user } + + it_behaves_like 'ref new comment' + it 'returns the inline comment' do - post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment', path: project.repository.commit.raw_diffs.first.new_path, line: 1, line_type: 'new' + post api(route, current_user), note: 'My comment', path: project.repository.commit.raw_diffs.first.new_path, line: 1, line_type: 'new' - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) + expect(response).to match_response_schema('public_api/v4/commit_note') expect(json_response['note']).to eq('My comment') expect(json_response['path']).to eq(project.repository.commit.raw_diffs.first.new_path) expect(json_response['line']).to eq(1) expect(json_response['line_type']).to eq('new') end + context 'when ref does not exist' do + let(:commit_id) { 'unknown' } + + it_behaves_like '404 response' do + let(:request) { post api(route, current_user), note: 'My comment' } + let(:message) { '404 Commit Not Found' } + end + end + it 'returns 400 if note is missing' do - post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user) - expect(response).to have_http_status(400) + post api(route, current_user) + + expect(response).to have_gitlab_http_status(400) end - it 'returns 404 if note is attached to non existent commit' do - post api("/projects/#{project.id}/repository/commits/1234ab/comments", user), note: 'My comment' - expect(response).to have_http_status(404) + context 'when ref contains a dot' do + let(:commit_id) { branch_with_dot.name } + + it_behaves_like 'ref new comment' end - end - context 'unauthorized user' do - it 'does not return the diff of the selected commit' do - post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments") - expect(response).to have_http_status(401) + context 'when ref contains a slash' do + let(:commit_id) { branch_with_slash.name } + + it_behaves_like '404 response' do + let(:request) { post api(route, current_user), note: 'My comment' } + end + end + + context 'when ref contains an escaped slash' do + let(:commit_id) { CGI.escape(branch_with_slash.name) } + + it_behaves_like 'ref new comment' + end + + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } + + it_behaves_like 'ref new comment' + + context 'when ref contains a dot' do + let(:commit_id) { branch_with_dot.name } + + it_behaves_like 'ref new comment' + end end end end diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index d032d72de9c..e497ec333a2 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -182,7 +182,7 @@ describe API::DeployKeys do delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) expect(response).to have_http_status(204) - end.to change{ project.deploy_keys.count }.by(-1) + end.to change { project.deploy_keys.count }.by(-1) end it 'returns 404 Not Found with invalid ID' do diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index 7a847442469..f1a26b6ce6c 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -138,5 +138,35 @@ describe API::Events do expect(response).to have_http_status(404) end end + + context 'when exists some events' do + let(:merge_request1) { create(:merge_request, :closed, author: user, assignee: user, source_project: private_project, title: 'Test') } + let(:merge_request2) { create(:merge_request, :closed, author: user, assignee: user, source_project: private_project, title: 'Test') } + + before do + create_event(merge_request1) + end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new do + get api("/projects/#{private_project.id}/events", user), target_type: :merge_request + end.count + + create_event(merge_request2) + + expect do + get api("/projects/#{private_project.id}/events", user), target_type: :merge_request + end.not_to exceed_query_limit(control_count) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response.size).to eq(2) + expect(json_response.map { |r| r['target_id'] }).to match_array([merge_request1.id, merge_request2.id]) + end + + def create_event(target) + create(:event, project: private_project, author: user, target: target) + end + end end end diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 402ea057cc5..2179790d098 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -88,7 +88,7 @@ describe API::GroupVariables do it 'creates variable' do expect do post api("/groups/#{group.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2', protected: true - end.to change{group.variables.count}.by(1) + end.to change {group.variables.count}.by(1) expect(response).to have_http_status(201) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -99,7 +99,7 @@ describe API::GroupVariables do it 'creates variable with optional attributes' do expect do post api("/groups/#{group.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2' - end.to change{group.variables.count}.by(1) + end.to change {group.variables.count}.by(1) expect(response).to have_http_status(201) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -110,7 +110,7 @@ describe API::GroupVariables do it 'does not allow to duplicate variable key' do expect do post api("/groups/#{group.id}/variables", user), key: variable.key, value: 'VALUE_2' - end.to change{group.variables.count}.by(0) + end.to change {group.variables.count}.by(0) expect(response).to have_http_status(400) end @@ -192,7 +192,7 @@ describe API::GroupVariables do delete api("/groups/#{group.id}/variables/#{variable.key}", user) expect(response).to have_http_status(204) - end.to change{group.variables.count}.by(-1) + end.to change {group.variables.count}.by(-1) end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 7a1bd76af7a..d4006fe71a2 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -56,7 +56,7 @@ describe API::Helpers do end def doorkeeper_guard_returns(value) - allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ value } + allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { value } end def error!(message, status, header) @@ -161,7 +161,7 @@ describe API::Helpers do describe "when authenticating using a user's private token" do it "returns nil for an invalid token" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token' - allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false } + allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false } expect(current_user).to be_nil end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 9eda6836ded..1e8eccb9b1c 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -326,7 +326,7 @@ describe API::MergeRequests do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end @@ -337,7 +337,7 @@ describe API::MergeRequests do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -348,7 +348,7 @@ describe API::MergeRequests do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['updated_at'] } + response_dates = json_response.map { |merge_request| merge_request['updated_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -359,7 +359,7 @@ describe API::MergeRequests do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end end diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 9ff2b782b52..1fc0ec528b9 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -53,7 +53,7 @@ describe API::PipelineSchedules do it 'returns matched pipeline schedules' do get api("/projects/#{project.id}/pipeline_schedules", developer), scope: target - expect(json_response.map{ |r| r['active'] }).to all(eq(active?(target))) + expect(json_response.map { |r| r['active'] }).to all(eq(active?(target))) end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index edd6516cf34..e9ee3dd679d 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -683,7 +683,7 @@ describe API::Runner do end context 'when job has been updated recently' do - it { expect{ patch_the_trace }.not_to change { job.updated_at }} + it { expect { patch_the_trace }.not_to change { job.updated_at }} it "changes the job's trace" do patch_the_trace @@ -692,7 +692,7 @@ describe API::Runner do end context 'when Runner makes a force-patch' do - it { expect{ force_patch_the_trace }.not_to change { job.updated_at }} + it { expect { force_patch_the_trace }.not_to change { job.updated_at }} it "doesn't change the build.trace" do force_patch_the_trace diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 3a95db030d4..c8ff25f70fa 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -36,7 +36,7 @@ describe API::Runners do it 'returns user available runners' do get api('/runners', user) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -46,7 +46,7 @@ describe API::Runners do it 'filters runners by scope' do get api('/runners?scope=active', user) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -74,7 +74,7 @@ describe API::Runners do it 'returns all runners' do get api('/runners/all', admin) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -93,7 +93,7 @@ describe API::Runners do it 'filters runners by scope' do get api('/runners/all?scope=specific', admin) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -277,7 +277,7 @@ describe API::Runners do delete api("/runners/#{shared_runner.id}", admin) expect(response).to have_http_status(204) - end.to change{ Ci::Runner.shared.count }.by(-1) + end.to change { Ci::Runner.shared.count }.by(-1) end end @@ -287,7 +287,7 @@ describe API::Runners do delete api("/runners/#{unused_specific_runner.id}", admin) expect(response).to have_http_status(204) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end it 'deletes used runner' do @@ -295,7 +295,7 @@ describe API::Runners do delete api("/runners/#{specific_runner.id}", admin) expect(response).to have_http_status(204) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end end @@ -330,7 +330,7 @@ describe API::Runners do delete api("/runners/#{specific_runner.id}", user) expect(response).to have_http_status(204) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end end end @@ -349,7 +349,7 @@ describe API::Runners do it "returns project's runners" do get api("/projects/#{project.id}/runners", user) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -385,14 +385,14 @@ describe API::Runners do it 'enables specific runner' do expect do post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id - end.to change{ project.runners.count }.by(+1) + end.to change { project.runners.count }.by(+1) expect(response).to have_http_status(201) end it 'avoids changes when enabling already enabled runner' do expect do post api("/projects/#{project.id}/runners", user), runner_id: specific_runner.id - end.to change{ project.runners.count }.by(0) + end.to change { project.runners.count }.by(0) expect(response).to have_http_status(409) end @@ -401,7 +401,7 @@ describe API::Runners do expect do post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id - end.to change{ project.runners.count }.by(0) + end.to change { project.runners.count }.by(0) expect(response).to have_http_status(403) end @@ -416,7 +416,7 @@ describe API::Runners do it 'enables any specific runner' do expect do post api("/projects/#{project.id}/runners", admin), runner_id: unused_specific_runner.id - end.to change{ project.runners.count }.by(+1) + end.to change { project.runners.count }.by(+1) expect(response).to have_http_status(201) end end @@ -461,7 +461,7 @@ describe API::Runners do delete api("/projects/#{project.id}/runners/#{two_projects_runner.id}", user) expect(response).to have_http_status(204) - end.to change{ project.runners.count }.by(-1) + end.to change { project.runners.count }.by(-1) end end @@ -469,7 +469,7 @@ describe API::Runners do it "does not disable project's runner" do expect do delete api("/projects/#{project.id}/runners/#{specific_runner.id}", user) - end.to change{ project.runners.count }.by(0) + end.to change { project.runners.count }.by(0) expect(response).to have_http_status(403) end end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 373fab4d98a..d09b8bc42f1 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -52,10 +52,10 @@ describe API::Snippets do expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( public_snippet.id, public_snippet_other.id) - expect(json_response.map{ |snippet| snippet['web_url']} ).to include( + expect(json_response.map { |snippet| snippet['web_url']} ).to include( "http://localhost/snippets/#{public_snippet.id}", "http://localhost/snippets/#{public_snippet_other.id}") - expect(json_response.map{ |snippet| snippet['raw_url']} ).to include( + expect(json_response.map { |snippet| snippet['raw_url']} ).to include( "http://localhost/snippets/#{public_snippet.id}/raw", "http://localhost/snippets/#{public_snippet_other.id}/raw") end diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index d5c53b703dd..572e9a0fd07 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -185,7 +185,7 @@ describe API::Triggers do expect do post api("/projects/#{project.id}/triggers", user), description: 'trigger' - end.to change{project.triggers.count}.by(1) + end.to change {project.triggers.count}.by(1) expect(response).to have_http_status(201) expect(json_response).to include('description' => 'trigger') @@ -288,7 +288,7 @@ describe API::Triggers do delete api("/projects/#{project.id}/triggers/#{trigger.id}", user) expect(response).to have_http_status(204) - end.to change{project.triggers.count}.by(-1) + end.to change {project.triggers.count}.by(-1) end it 'responds with 404 Not Found if requesting non-existing trigger' do diff --git a/spec/requests/api/v3/deploy_keys_spec.rb b/spec/requests/api/v3/deploy_keys_spec.rb index 13a62423b1d..2affd0cfa51 100644 --- a/spec/requests/api/v3/deploy_keys_spec.rb +++ b/spec/requests/api/v3/deploy_keys_spec.rb @@ -87,7 +87,7 @@ describe API::V3::DeployKeys do expect do post v3_api("/projects/#{project.id}/#{path}", admin), key_attrs - end.to change{ project.deploy_keys.count }.by(1) + end.to change { project.deploy_keys.count }.by(1) end it 'returns an existing ssh key when attempting to add a duplicate' do @@ -122,7 +122,7 @@ describe API::V3::DeployKeys do it 'should delete existing key' do expect do delete v3_api("/projects/#{project.id}/#{path}/#{deploy_key.id}", admin) - end.to change{ project.deploy_keys.count }.by(-1) + end.to change { project.deploy_keys.count }.by(-1) end it 'should return 404 Not Found with invalid ID' do diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index ef7516fc28f..18d0a804137 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -90,7 +90,7 @@ describe API::MergeRequests do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end @@ -99,7 +99,7 @@ describe API::MergeRequests do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -108,7 +108,7 @@ describe API::MergeRequests do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['updated_at'] } + response_dates = json_response.map { |merge_request| merge_request['updated_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -117,7 +117,7 @@ describe API::MergeRequests do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end end diff --git a/spec/requests/api/v3/project_snippets_spec.rb b/spec/requests/api/v3/project_snippets_spec.rb index 758fb482374..3963924a066 100644 --- a/spec/requests/api/v3/project_snippets_spec.rb +++ b/spec/requests/api/v3/project_snippets_spec.rb @@ -30,7 +30,7 @@ describe API::ProjectSnippets do expect(response).to have_http_status(200) expect(json_response.size).to eq(3) - expect(json_response.map{ |snippet| snippet['id']} ).to include(public_snippet.id, internal_snippet.id, private_snippet.id) + expect(json_response.map { |snippet| snippet['id']} ).to include(public_snippet.id, internal_snippet.id, private_snippet.id) expect(json_response.last).to have_key('web_url') end diff --git a/spec/requests/api/v3/runners_spec.rb b/spec/requests/api/v3/runners_spec.rb index 78660afd840..a31eb3f1d43 100644 --- a/spec/requests/api/v3/runners_spec.rb +++ b/spec/requests/api/v3/runners_spec.rb @@ -38,7 +38,7 @@ describe API::V3::Runners do delete v3_api("/runners/#{shared_runner.id}", admin) expect(response).to have_http_status(200) - end.to change{ Ci::Runner.shared.count }.by(-1) + end.to change { Ci::Runner.shared.count }.by(-1) end end @@ -48,7 +48,7 @@ describe API::V3::Runners do delete v3_api("/runners/#{unused_specific_runner.id}", admin) expect(response).to have_http_status(200) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end it 'deletes used runner' do @@ -56,7 +56,7 @@ describe API::V3::Runners do delete v3_api("/runners/#{specific_runner.id}", admin) expect(response).to have_http_status(200) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end end @@ -91,7 +91,7 @@ describe API::V3::Runners do delete v3_api("/runners/#{specific_runner.id}", user) expect(response).to have_http_status(200) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end end end @@ -113,7 +113,7 @@ describe API::V3::Runners do delete v3_api("/projects/#{project.id}/runners/#{two_projects_runner.id}", user) expect(response).to have_http_status(200) - end.to change{ project.runners.count }.by(-1) + end.to change { project.runners.count }.by(-1) end end @@ -121,7 +121,7 @@ describe API::V3::Runners do it "does not disable project's runner" do expect do delete v3_api("/projects/#{project.id}/runners/#{specific_runner.id}", user) - end.to change{ project.runners.count }.by(0) + end.to change { project.runners.count }.by(0) expect(response).to have_http_status(403) end end diff --git a/spec/requests/api/v3/snippets_spec.rb b/spec/requests/api/v3/snippets_spec.rb index 1bc2258ebd3..9ead3cad8bb 100644 --- a/spec/requests/api/v3/snippets_spec.rb +++ b/spec/requests/api/v3/snippets_spec.rb @@ -45,10 +45,10 @@ describe API::V3::Snippets do expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( public_snippet.id, public_snippet_other.id) - expect(json_response.map{ |snippet| snippet['web_url']} ).to include( + expect(json_response.map { |snippet| snippet['web_url']} ).to include( "http://localhost/snippets/#{public_snippet.id}", "http://localhost/snippets/#{public_snippet_other.id}") - expect(json_response.map{ |snippet| snippet['raw_url']} ).to include( + expect(json_response.map { |snippet| snippet['raw_url']} ).to include( "http://localhost/snippets/#{public_snippet.id}/raw", "http://localhost/snippets/#{public_snippet_other.id}/raw") end diff --git a/spec/requests/api/v3/triggers_spec.rb b/spec/requests/api/v3/triggers_spec.rb index 60212660fb6..075de2c0cba 100644 --- a/spec/requests/api/v3/triggers_spec.rb +++ b/spec/requests/api/v3/triggers_spec.rb @@ -171,7 +171,7 @@ describe API::V3::Triggers do it 'creates trigger' do expect do post v3_api("/projects/#{project.id}/triggers", user) - end.to change{project.triggers.count}.by(1) + end.to change {project.triggers.count}.by(1) expect(response).to have_http_status(201) expect(json_response).to be_a(Hash) @@ -202,7 +202,7 @@ describe API::V3::Triggers do delete v3_api("/projects/#{project.id}/triggers/#{trigger.token}", user) expect(response).to have_http_status(200) - end.to change{project.triggers.count}.by(-1) + end.to change {project.triggers.count}.by(-1) end it 'responds with 404 Not Found if requesting non-existing trigger' do diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 098a0d8ca7d..48592e12822 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -74,7 +74,7 @@ describe API::Variables do it 'creates variable' do expect do post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2', protected: true - end.to change{project.variables.count}.by(1) + end.to change {project.variables.count}.by(1) expect(response).to have_http_status(201) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -85,7 +85,7 @@ describe API::Variables do it 'creates variable with optional attributes' do expect do post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2' - end.to change{project.variables.count}.by(1) + end.to change {project.variables.count}.by(1) expect(response).to have_http_status(201) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -96,7 +96,7 @@ describe API::Variables do it 'does not allow to duplicate variable key' do expect do post api("/projects/#{project.id}/variables", user), key: variable.key, value: 'VALUE_2' - end.to change{project.variables.count}.by(0) + end.to change {project.variables.count}.by(0) expect(response).to have_http_status(400) end @@ -166,7 +166,7 @@ describe API::Variables do delete api("/projects/#{project.id}/variables/#{variable.key}", user) expect(response).to have_http_status(204) - end.to change{project.variables.count}.by(-1) + end.to change {project.variables.count}.by(-1) end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index c077c458163..ebd67eb1e94 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -421,7 +421,7 @@ describe Ci::API::Builds do end context 'when build has been updated recently' do - it { expect{ patch_the_trace }.not_to change { build.updated_at }} + it { expect { patch_the_trace }.not_to change { build.updated_at }} it 'changes the build trace' do patch_the_trace @@ -430,7 +430,7 @@ describe Ci::API::Builds do end context 'when Runner makes a force-patch' do - it { expect{ force_patch_the_trace }.not_to change { build.updated_at }} + it { expect { force_patch_the_trace }.not_to change { build.updated_at }} it "doesn't change the build.trace" do force_patch_the_trace diff --git a/spec/serializers/analytics_build_entity_spec.rb b/spec/serializers/analytics_build_entity_spec.rb index 86e703a6448..9f26d5cd09a 100644 --- a/spec/serializers/analytics_build_entity_spec.rb +++ b/spec/serializers/analytics_build_entity_spec.rb @@ -47,7 +47,7 @@ describe AnalyticsBuildEntity do let(:finished_at) { nil } it 'does not blow up' do - expect{ subject[:date] }.not_to raise_error + expect { subject[:date] }.not_to raise_error end it 'shows the right message' do @@ -63,7 +63,7 @@ describe AnalyticsBuildEntity do let(:started_at) { nil } it 'does not blow up' do - expect{ subject[:date] }.not_to raise_error + expect { subject[:date] }.not_to raise_error end it 'shows the right message' do @@ -79,7 +79,7 @@ describe AnalyticsBuildEntity do let(:finished_at) { nil } it 'does not blow up' do - expect{ subject[:date] }.not_to raise_error + expect { subject[:date] }.not_to raise_error end it 'shows the right message' do diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 362d754bca3..2de8daba6b5 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -111,7 +111,7 @@ describe PipelineSerializer do shared_examples 'no N+1 queries' do it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(1).of(59) + expect(recorded.count).to be_within(1).of(57) expect(recorded.cached_count).to eq(0) end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 82724ccd281..a6449a3c9f5 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -132,7 +132,7 @@ describe GitPushService, services: true do end it "creates a new pipeline" do - expect{ subject }.to change{ Ci::Pipeline.count } + expect { subject }.to change { Ci::Pipeline.count } expect(Ci::Pipeline.last).to be_push end end diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index f877c145390..05695aa8188 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -39,7 +39,7 @@ describe GitTagPushService do end it "creates a new pipeline" do - expect{ subject }.to change{ Ci::Pipeline.count } + expect { subject }.to change { Ci::Pipeline.count } expect(Ci::Pipeline.last).to be_push end end diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index befa65ec0f8..bdaab88d673 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -118,7 +118,7 @@ describe Issuable::BulkUpdateService do context 'when the new assignee ID is not present' do it 'does not unassign' do expect { bulk_update(issue, assignee_ids: []) } - .not_to change{ issue.reload.assignees } + .not_to change { issue.reload.assignees } end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index fcbd69fd58b..9b2d9e79f4f 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -351,14 +351,14 @@ describe Issues::CreateService do end it 'marks related spam_log as recaptcha_verified' do - expect { issue }.to change{SpamLog.last.recaptcha_verified}.from(false).to(true) + expect { issue }.to change {SpamLog.last.recaptcha_verified}.from(false).to(true) end context 'when spam log does not belong to a user' do let(:log_user) { create(:user) } it 'does not mark spam_log as recaptcha_verified' do - expect { issue }.not_to change{SpamLog.last.recaptcha_verified} + expect { issue }.not_to change {SpamLog.last.recaptcha_verified} end end end @@ -378,7 +378,7 @@ describe Issues::CreateService do end it 'creates a new spam_log' do - expect{issue}.to change{SpamLog.count}.from(0).to(1) + expect {issue}.to change {SpamLog.count}.from(0).to(1) end it 'assigns a spam_log to an issue' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 814e2cfbed0..34fb16edc84 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -478,7 +478,7 @@ describe Issues::UpdateService, :mailer do feature_visibility_attr = :"#{issue.model_name.plural}_access_level" project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) - expect{ update_issue(assignee_ids: [assignee.id]) }.not_to change{ issue.assignees } + expect { update_issue(assignee_ids: [assignee.id]) }.not_to change { issue.assignees } end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 9368594bc86..681feee61d1 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -509,7 +509,7 @@ describe MergeRequests::UpdateService, :mailer do feature_visibility_attr = :"#{merge_request.model_name.plural}_access_level" project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) - expect{ update_merge_request(assignee_id: assignee) }.not_to change{ merge_request.assignee } + expect { update_merge_request(assignee_id: assignee) }.not_to change { merge_request.assignee } end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 6af5c79135d..bd8ff5aaaa7 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -85,7 +85,7 @@ describe NotificationService, :mailer do it { expect(notification.new_key(key)).to be_truthy } it 'sends email to key owner' do - expect{ notification.new_key(key) }.to change{ ActionMailer::Base.deliveries.size }.by(1) + expect { notification.new_key(key) }.to change { ActionMailer::Base.deliveries.size }.by(1) end end end @@ -97,7 +97,7 @@ describe NotificationService, :mailer do it { expect(notification.new_gpg_key(key)).to be_truthy } it 'sends email to key owner' do - expect{ notification.new_gpg_key(key) }.to change{ ActionMailer::Base.deliveries.size }.by(1) + expect { notification.new_gpg_key(key) }.to change { ActionMailer::Base.deliveries.size }.by(1) end end end @@ -109,7 +109,7 @@ describe NotificationService, :mailer do it { expect(notification.new_email(email)).to be_truthy } it 'sends email to email owner' do - expect{ notification.new_email(email) }.to change{ ActionMailer::Base.deliveries.size }.by(1) + expect { notification.new_email(email) }.to change { ActionMailer::Base.deliveries.size }.by(1) end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index d945e0aa1f0..1b282e82187 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -101,6 +101,13 @@ describe Projects::UpdateService, '#execute' do expect(Project.find(project.id).default_branch).to eq 'feature' end + + it 'does not change a default branch' do + # The branch 'unexisted-branch' does not exist. + update_project(project, admin, default_branch: 'unexisted-branch') + + expect(Project.find(project.id).default_branch).to eq 'master' + end end context 'when updating a project that contains container images' do diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb index 5698101af54..9fa5983db66 100644 --- a/spec/services/protected_branches/update_service_spec.rb +++ b/spec/services/protected_branches/update_service_spec.rb @@ -19,7 +19,7 @@ describe ProtectedBranches::UpdateService do let(:user) { create(:user) } it "raises error" do - expect{ service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError) + expect { service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError) end end end diff --git a/spec/services/protected_tags/update_service_spec.rb b/spec/services/protected_tags/update_service_spec.rb index d333430088d..2ece4e3b07b 100644 --- a/spec/services/protected_tags/update_service_spec.rb +++ b/spec/services/protected_tags/update_service_spec.rb @@ -19,7 +19,7 @@ describe ProtectedTags::UpdateService do let(:user) { create(:user) } it "raises error" do - expect{ service.execute(protected_tag) }.to raise_error(Gitlab::Access::AccessDeniedError) + expect { service.execute(protected_tag) }.to raise_error(Gitlab::Access::AccessDeniedError) end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 8f1eb4863d9..6d36affa9dc 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -35,7 +35,7 @@ describe SystemNoteService do context 'metadata' do it 'creates a new system note metadata record' do - expect { subject }.to change{ SystemNoteMetadata.count }.from(0).to(1) + expect { subject }.to change { SystemNoteMetadata.count }.from(0).to(1) end it 'creates a record correctly' do @@ -477,7 +477,7 @@ describe SystemNoteService do end it 'does not create a system note metadata record' do - expect { subject }.not_to change{ SystemNoteMetadata.count } + expect { subject }.not_to change { SystemNoteMetadata.count } end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 80d05451e09..a9b34a5258a 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -586,13 +586,13 @@ describe TodoService do it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do create(:todo, :directly_addressed, user: member, project: project, target: addressed_mr_assigned, author: author) - expect{ service.update_merge_request(addressed_mr_assigned, author) }.not_to change(member.todos, :count) + expect { service.update_merge_request(addressed_mr_assigned, author) }.not_to change(member.todos, :count) end it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is done' do create(:todo, :directly_addressed, user: skipped, project: project, target: addressed_mr_assigned, author: author) - expect{ service.update_merge_request(addressed_mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count) + expect { service.update_merge_request(addressed_mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count) end context 'with a task list' do diff --git a/spec/support/api/milestones_shared_examples.rb b/spec/support/api/milestones_shared_examples.rb index bf769225012..4bb5113957e 100644 --- a/spec/support/api/milestones_shared_examples.rb +++ b/spec/support/api/milestones_shared_examples.rb @@ -50,7 +50,7 @@ shared_examples_for 'group and project milestones' do |route_definition| expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(2) - expect(json_response.map{ |m| m['id'] }).to match_array([closed_milestone.id, other_milestone.id]) + expect(json_response.map { |m| m['id'] }).to match_array([closed_milestone.id, other_milestone.id]) end it 'does not return any milestone if none found' do diff --git a/spec/support/matchers/query_matcher.rb b/spec/support/matchers/query_matcher.rb index ac8c4ab91d9..bb0feca7c43 100644 --- a/spec/support/matchers/query_matcher.rb +++ b/spec/support/matchers/query_matcher.rb @@ -28,6 +28,6 @@ RSpec::Matchers.define :make_queries_matching do |matcher, expected_count = nil| def query_count(regex, &block) @recorder = ActiveRecord::QueryRecorder.new(&block).log - @recorder.select{ |q| q.match(regex) } + @recorder.select { |q| q.match(regex) } end end diff --git a/spec/support/rake_helpers.rb b/spec/support/rake_helpers.rb index 5cb415111d2..86bfeed107c 100644 --- a/spec/support/rake_helpers.rb +++ b/spec/support/rake_helpers.rb @@ -5,11 +5,15 @@ module RakeHelpers end def stub_warn_user_is_not_gitlab - allow_any_instance_of(Object).to receive(:warn_user_is_not_gitlab) + allow(main_object).to receive(:warn_user_is_not_gitlab) end def silence_output - allow($stdout).to receive(:puts) - allow($stdout).to receive(:print) + allow(main_object).to receive(:puts) + allow(main_object).to receive(:print) + end + + def main_object + @main_object ||= TOPLEVEL_BINDING.eval('self') end end diff --git a/spec/support/stub_configuration.rb b/spec/support/stub_configuration.rb index 516f8878679..37c89d37aa0 100644 --- a/spec/support/stub_configuration.rb +++ b/spec/support/stub_configuration.rb @@ -38,6 +38,17 @@ module StubConfiguration allow(Gitlab.config.backup).to receive_messages(to_settings(messages)) end + def stub_storage_settings(messages) + messages.each do |storage_name, storage_settings| + storage_settings['failure_count_threshold'] ||= 10 + storage_settings['failure_wait_time'] ||= 30 + storage_settings['failure_reset_time'] ||= 1800 + storage_settings['storage_timeout'] ||= 5 + end + + allow(Gitlab.config.repositories).to receive(:storages).and_return(messages) + end + private # Modifies stubbed messages to also stub possible predicate versions diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index e059c9620ab..1e39f80699c 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -250,6 +250,14 @@ module TestEnv "#{forked_repo_path}_bare" end + def with_empty_bare_repository(name = nil) + path = Rails.root.join('tmp/tests', name || 'empty-bare-repository').to_s + + yield(Rugged::Repository.init_at(path, :bare)) + ensure + FileUtils.rm_rf(path) + end + private def factory_repo_path diff --git a/spec/tasks/config_lint_spec.rb b/spec/tasks/config_lint_spec.rb index ed6c5b09663..5b01665019a 100644 --- a/spec/tasks/config_lint_spec.rb +++ b/spec/tasks/config_lint_spec.rb @@ -2,14 +2,14 @@ require 'rake_helper' Rake.application.rake_require 'tasks/config_lint' describe ConfigLint do - let(:files){ ['lib/support/fake.sh'] } + let(:files) { ['lib/support/fake.sh'] } it 'errors out if any bash scripts have errors' do - expect { described_class.run(files){ system('exit 1') } }.to raise_error(SystemExit) + expect { described_class.run(files) { system('exit 1') } }.to raise_error(SystemExit) end it 'passes if all scripts are fine' do - expect { described_class.run(files){ system('exit 0') } }.not_to raise_error + expect { described_class.run(files) { system('exit 0') } }.not_to raise_error end end diff --git a/spec/tasks/gitlab/gitaly_rake_spec.rb b/spec/tasks/gitlab/gitaly_rake_spec.rb index cc932a4ec4c..43ac1a72152 100644 --- a/spec/tasks/gitlab/gitaly_rake_spec.rb +++ b/spec/tasks/gitlab/gitaly_rake_spec.rb @@ -20,7 +20,7 @@ describe 'gitlab:gitaly namespace rake task' do context 'when an underlying Git command fail' do it 'aborts and display a help message' do - expect_any_instance_of(Object) + expect(main_object) .to receive(:checkout_or_clone_version).and_raise 'Git error' expect { run_rake_task('gitlab:gitaly:install', clone_path) }.to raise_error 'Git error' @@ -33,7 +33,7 @@ describe 'gitlab:gitaly namespace rake task' do end it 'calls checkout_or_clone_version with the right arguments' do - expect_any_instance_of(Object) + expect(main_object) .to receive(:checkout_or_clone_version).with(version: version, repo: repo, target_dir: clone_path) run_rake_task('gitlab:gitaly:install', clone_path) @@ -41,7 +41,7 @@ describe 'gitlab:gitaly namespace rake task' do end describe 'gmake/make' do - let(:command_preamble) { %w[/usr/bin/env -u RUBYOPT] } + let(:command_preamble) { %w[/usr/bin/env -u RUBYOPT -u BUNDLE_GEMFILE] } before(:all) do @old_env_ci = ENV.delete('CI') @@ -58,13 +58,13 @@ describe 'gitlab:gitaly namespace rake task' do context 'gmake is available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) + expect(main_object).to receive(:checkout_or_clone_version) + allow(main_object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) end it 'calls gmake in the gitaly directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['/usr/bin/gmake', 0]) - expect_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) + expect(main_object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end @@ -72,13 +72,13 @@ describe 'gitlab:gitaly namespace rake task' do context 'gmake is not available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) + expect(main_object).to receive(:checkout_or_clone_version) + allow(main_object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) end it 'calls make in the gitaly directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['', 42]) - expect_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) + expect(main_object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end diff --git a/spec/tasks/gitlab/shell_rake_spec.rb b/spec/tasks/gitlab/shell_rake_spec.rb index ee3614c50f6..65155cb044d 100644 --- a/spec/tasks/gitlab/shell_rake_spec.rb +++ b/spec/tasks/gitlab/shell_rake_spec.rb @@ -22,7 +22,8 @@ describe 'gitlab:shell rake tasks' do describe 'create_hooks task' do it 'calls gitlab-shell bin/create_hooks' do expect_any_instance_of(Object).to receive(:system) - .with("#{Gitlab.config.gitlab_shell.path}/bin/create-hooks", *repository_storage_paths_args) + .with("#{Gitlab.config.gitlab_shell.path}/bin/create-hooks", + *Gitlab::TaskHelpers.repository_storage_paths_args) run_rake_task('gitlab:shell:create_hooks') end diff --git a/spec/tasks/gitlab/workhorse_rake_spec.rb b/spec/tasks/gitlab/workhorse_rake_spec.rb index 1b68f3044a4..42516d36c67 100644 --- a/spec/tasks/gitlab/workhorse_rake_spec.rb +++ b/spec/tasks/gitlab/workhorse_rake_spec.rb @@ -20,7 +20,7 @@ describe 'gitlab:workhorse namespace rake task' do context 'when an underlying Git command fail' do it 'aborts and display a help message' do - expect_any_instance_of(Object) + expect(main_object) .to receive(:checkout_or_clone_version).and_raise 'Git error' expect { run_rake_task('gitlab:workhorse:install', clone_path) }.to raise_error 'Git error' @@ -33,7 +33,7 @@ describe 'gitlab:workhorse namespace rake task' do end it 'calls checkout_or_clone_version with the right arguments' do - expect_any_instance_of(Object) + expect(main_object) .to receive(:checkout_or_clone_version).with(version: version, repo: repo, target_dir: clone_path) run_rake_task('gitlab:workhorse:install', clone_path) @@ -48,13 +48,13 @@ describe 'gitlab:workhorse namespace rake task' do context 'gmake is available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true) + expect(main_object).to receive(:checkout_or_clone_version) + allow(Object).to receive(:run_command!).with(['gmake']).and_return(true) end it 'calls gmake in the gitlab-workhorse directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['/usr/bin/gmake', 0]) - expect_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true) + expect(main_object).to receive(:run_command!).with(['gmake']).and_return(true) run_rake_task('gitlab:workhorse:install', clone_path) end @@ -62,13 +62,13 @@ describe 'gitlab:workhorse namespace rake task' do context 'gmake is not available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true) + expect(main_object).to receive(:checkout_or_clone_version) + allow(main_object).to receive(:run_command!).with(['make']).and_return(true) end it 'calls make in the gitlab-workhorse directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['', 42]) - expect_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true) + expect(main_object).to receive(:run_command!).with(['make']).and_return(true) run_rake_task('gitlab:workhorse:install', clone_path) end diff --git a/spec/workers/new_issue_worker_spec.rb b/spec/workers/new_issue_worker_spec.rb index ed49ce57c0b..4e15ccc534b 100644 --- a/spec/workers/new_issue_worker_spec.rb +++ b/spec/workers/new_issue_worker_spec.rb @@ -41,7 +41,7 @@ describe NewIssueWorker do let(:issue) { create(:issue, project: project, description: "issue for #{mentioned.to_reference}") } it 'creates a new event record' do - expect{ worker.perform(issue.id, user.id) }.to change { Event.count }.from(0).to(1) + expect { worker.perform(issue.id, user.id) }.to change { Event.count }.from(0).to(1) end it 'creates a notification for the assignee' do diff --git a/spec/workers/new_merge_request_worker_spec.rb b/spec/workers/new_merge_request_worker_spec.rb index 85af6184d39..9e0cbde45b1 100644 --- a/spec/workers/new_merge_request_worker_spec.rb +++ b/spec/workers/new_merge_request_worker_spec.rb @@ -43,7 +43,7 @@ describe NewMergeRequestWorker do end it 'creates a new event record' do - expect{ worker.perform(merge_request.id, user.id) }.to change { Event.count }.from(0).to(1) + expect { worker.perform(merge_request.id, user.id) }.to change { Event.count }.from(0).to(1) end it 'creates a notification for the assignee' do diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb index 14ed8b7811e..75197039f5a 100644 --- a/spec/workers/pipeline_schedule_worker_spec.rb +++ b/spec/workers/pipeline_schedule_worker_spec.rb @@ -23,7 +23,7 @@ describe PipelineScheduleWorker do context 'when there is a scheduled pipeline within next_run_at' do it 'creates a new pipeline' do - expect{ subject }.to change { project.pipelines.count }.by(1) + expect { subject }.to change { project.pipelines.count }.by(1) expect(Ci::Pipeline.last).to be_schedule end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 74a9f90195c..af6a3c9f6c7 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -78,7 +78,7 @@ describe PostReceive do stub_ci_pipeline_to_return_yaml_file end - it { expect{ subject }.to change{ Ci::Pipeline.count }.by(2) } + it { expect { subject }.to change { Ci::Pipeline.count }.by(2) } end context "does not create a Ci::Pipeline" do @@ -86,7 +86,7 @@ describe PostReceive do stub_ci_pipeline_yaml_file(nil) end - it { expect{ subject }.not_to change{ Ci::Pipeline.count } } + it { expect { subject }.not_to change { Ci::Pipeline.count } } end end |