diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-27 06:11:24 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-27 06:11:24 +0300 |
commit | 729e66ee8e5790eefb3771040839155c499faab3 (patch) | |
tree | 19cd1fd73c2373c20d2888e857de022129a79eaa | |
parent | 431b84710e87a649de02d398568804f820e3b0fe (diff) |
Add latest changes from gitlab-org/gitlab@master
44 files changed, 972 insertions, 266 deletions
@@ -392,7 +392,7 @@ group :development, :test do gem 'database_cleaner', '~> 1.7.0' gem 'factory_bot_rails', '~> 6.2.0' - gem 'rspec-rails', '~> 5.0.1' + gem 'rspec-rails', '~> 6.0.1' # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826) gem 'minitest', '~> 5.11.0' diff --git a/Gemfile.checksum b/Gemfile.checksum index 3a64a1d1007..e3f4b2d7b81 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -83,7 +83,7 @@ {"name":"coderay","version":"1.1.3","platform":"ruby","checksum":"dc530018a4684512f8f38143cd2a096c9f02a1fc2459edcfe534787a7fc77d4b"}, {"name":"colored2","version":"3.1.2","platform":"ruby","checksum":"b13c2bd7eeae2cf7356a62501d398e72fde78780bd26aec6a979578293c28b4a"}, {"name":"commonmarker","version":"0.23.6","platform":"ruby","checksum":"c8aeaaaff4ba497bf180f762db63a0069794fafb6eff221224c9c8199d337b38"}, -{"name":"concurrent-ruby","version":"1.1.10","platform":"ruby","checksum":"244cb1ca0d91ec2c15ca2209507c39fb163336994428e16fbd3f465c87bd8e68"}, +{"name":"concurrent-ruby","version":"1.2.0","platform":"ruby","checksum":"a5e799f71e7490f24a534d58c91380267d0ae306af0cdc518d6848b93475dae2"}, {"name":"connection_pool","version":"2.3.0","platform":"ruby","checksum":"677985be912f33c90f98f229aaa0c0ddb2ef8776f21929a36eeeb25251c944da"}, {"name":"cork","version":"0.3.0","platform":"ruby","checksum":"a0a0ac50e262f8514d1abe0a14e95e71c98b24e3378690e5d044daf0013ad4bc"}, {"name":"cose","version":"1.0.0","platform":"ruby","checksum":"520ebaad97b56d2873de02ff4e2c973f5e77ce2f8edbda454af9ee3073643bc0"}, @@ -136,7 +136,7 @@ {"name":"email_reply_trimmer","version":"0.1.6","platform":"ruby","checksum":"9fede222ce660993e4e2e3dad282535ceb7914e246eb8302c19aa9e021f7326e"}, {"name":"email_spec","version":"2.2.0","platform":"ruby","checksum":"60b7980580a835e7f676db60667f17a2d60e8e0e39c26d81cfc231805c544d79"}, {"name":"encryptor","version":"3.0.0","platform":"ruby","checksum":"abf23f94ab4d864b8cea85b43f3432044a60001982cda7c33c1cd90da8db1969"}, -{"name":"erubi","version":"1.11.0","platform":"ruby","checksum":"fda72d577feaf3bdcd646d33fa630be5f92f48e179a9278e4175a9cec20e7f85"}, +{"name":"erubi","version":"1.12.0","platform":"ruby","checksum":"27bedb74dfb1e04ff60674975e182d8ca787f2224f2e8143268c7696f42e4723"}, {"name":"escape_utils","version":"1.2.1","platform":"ruby","checksum":"e5292fe8d7e12a9bcb4502d99e28fb602e4e1514690d98a1c4957f6f77b4b162"}, {"name":"et-orbi","version":"1.2.7","platform":"ruby","checksum":"3b693d47f94a4060ccc07e60adda488759b1e8b9228a633ebbad842dfc245fb4"}, {"name":"ethon","version":"0.15.0","platform":"ruby","checksum":"0809805a035bc10f54162ca99f15ded49e428e0488bcfe1c08c821e18261a74d"}, @@ -336,7 +336,7 @@ {"name":"mini_histogram","version":"0.3.1","platform":"ruby","checksum":"6a114b504e4618b0e076cc672996036870f7cc6f16b8e5c25c0c637726d2dd94"}, {"name":"mini_magick","version":"4.10.1","platform":"ruby","checksum":"e939d2c70c8002233fc6b1eecfe762f38a156d69ad31a87160205870be08f852"}, {"name":"mini_mime","version":"1.1.2","platform":"ruby","checksum":"a54aec0cc7438a03a850adb00daca2bdb60747f839e28186994df057cea87151"}, -{"name":"mini_portile2","version":"2.8.0","platform":"ruby","checksum":"1e06b286ff19b73cfc9193cb3dd2bd80416f8262443564b25b23baea74a05765"}, +{"name":"mini_portile2","version":"2.8.1","platform":"ruby","checksum":"b70e325e37a378aea68b6d78c9cdd060c66cbd2bef558d8f13a6af05b3f2c4a9"}, {"name":"minitest","version":"5.11.3","platform":"ruby","checksum":"78e18aa2c49c58e9bc53c54a0b900e87ad0a96394e92fbbfa58d3ff860a68f45"}, {"name":"mixlib-cli","version":"2.1.8","platform":"ruby","checksum":"e6f27be34d580f6ed71731ca46b967e57793a627131c1f6e1ed2dad39ea3bdf9"}, {"name":"mixlib-config","version":"3.0.9","platform":"ruby","checksum":"9867adab3ab547eb74a8efdc9dfab6bcc83d2802a571ff8af8d6e981ca8d53ab"}, @@ -453,7 +453,7 @@ {"name":"rails","version":"6.1.7.1","platform":"ruby","checksum":"ad209c9822b494e0f0ff9faa68381bf209e4b7ca7d2e3e65f08a842e750163f7"}, {"name":"rails-controller-testing","version":"1.0.5","platform":"ruby","checksum":"741448db59366073e86fc965ba403f881c636b79a2c39a48d0486f2607182e94"}, {"name":"rails-dom-testing","version":"2.0.3","platform":"ruby","checksum":"b140c4f39f6e609c8113137b9a60dfc2ecb89864e496f87f23a68b3b8f12d8d1"}, -{"name":"rails-html-sanitizer","version":"1.4.4","platform":"ruby","checksum":"895d0c87a2b6623891e85c1d507c7f16acda4e77d94692f537df35ba71398bd5"}, +{"name":"rails-html-sanitizer","version":"1.5.0","platform":"ruby","checksum":"bf326075e8a968cd882c30b15a4c9100059be3af2356093dc68324ec3bd9ea79"}, {"name":"rails-i18n","version":"7.0.3","platform":"ruby","checksum":"e3158e98c5332d129fd5131f171ac575eb30dbb8919b21595382b08850cf2bd3"}, {"name":"railties","version":"6.1.7.1","platform":"ruby","checksum":"11b71777773e12bac44dc439e7e82e3e60787f7a1ca477e385acd9b689eb8e83"}, {"name":"rainbow","version":"3.1.1","platform":"ruby","checksum":"039491aa3a89f42efa1d6dec2fc4e62ede96eb6acd95e52f1ad581182b79bc6a"}, @@ -461,7 +461,7 @@ {"name":"rb-fsevent","version":"0.11.2","platform":"ruby","checksum":"43900b972e7301d6570f64b850a5aa67833ee7d87b458ee92805d56b7318aefe"}, {"name":"rb-inotify","version":"0.10.1","platform":"ruby","checksum":"050062d4f31d307cca52c3f6a7f4b946df8de25fc4bd373e1a5142e41034a7ca"}, {"name":"rbtrace","version":"0.4.14","platform":"ruby","checksum":"162bbf89cecabfc4f09c869b655f6f3a679c4870ebb7cbdcadf7393a81cc1769"}, -{"name":"rbtree","version":"0.4.4","platform":"ruby","checksum":"c1277a502a96fe8fd8656cb619db1ac87145df809ea4db35f7242b50bb161d5c"}, +{"name":"rbtree","version":"0.4.6","platform":"ruby","checksum":"14eea4469b24fd2472542e5f3eb105d6344c8ccf36f0b56d55fdcfeb4e0f10fc"}, {"name":"rchardet","version":"1.8.0","platform":"ruby","checksum":"693acd5253d5ade81a51940697955f6dd4bb2f0d245bda76a8e23deec70a52c7"}, {"name":"rdoc","version":"6.3.2","platform":"ruby","checksum":"def4a720235c27d56c176ae73555e647eb04ea58a8bbaa927f8f9f79de7805a6"}, {"name":"re2","version":"1.6.0","platform":"ruby","checksum":"2e37f27971f6a76223eac688c04f3e48aea374f34b302ec22d75b4635cd64bc1"}, @@ -491,15 +491,15 @@ {"name":"rouge","version":"3.30.0","platform":"ruby","checksum":"a3d353222aa72e49e2c86726c0bcfd719f82592f57d494474655f48e669eceb6"}, {"name":"rqrcode","version":"0.7.0","platform":"ruby","checksum":"8b3a5cba9cc199ba2d781a7c767cb55679f29a3621aa0506a799cec3760d16a1"}, {"name":"rqrcode-rails3","version":"0.1.7","platform":"ruby","checksum":"6f0582f26485123e5ed6f2a8a2871f00d86d353e0f58c8429a5a13212bcf48c4"}, -{"name":"rspec","version":"3.10.0","platform":"ruby","checksum":"b870b43d49ae4a4e063b94976d2742b0854ec10458c425d569b5556ee5898ab7"}, +{"name":"rspec","version":"3.12.0","platform":"ruby","checksum":"ccc41799a43509dc0be84070e3f0410ac95cbd480ae7b6c245543eb64162399c"}, {"name":"rspec-benchmark","version":"0.6.0","platform":"ruby","checksum":"1014adb57ec2599a2455c63884229f367a2fff6a63a77fd68ce5d804c83dd6cf"}, -{"name":"rspec-core","version":"3.10.2","platform":"ruby","checksum":"005659ce9dd356dd5d2acb4bcdcc5915291f4a312447b500af3b75aab564951b"}, -{"name":"rspec-expectations","version":"3.10.1","platform":"ruby","checksum":"27acf5d5df13f8cc8f7158001ebf572513bcec3d45404ba76e0a8998895ce9eb"}, -{"name":"rspec-mocks","version":"3.10.3","platform":"ruby","checksum":"d2f6f3d8b7569b1e846703d164cb23e24c7f530d38217fc06da2beaf6024260a"}, +{"name":"rspec-core","version":"3.12.0","platform":"ruby","checksum":"c466f4137966526e177d2156ca45c249eeecc7ed519b23ae2fb80c4675406bc5"}, +{"name":"rspec-expectations","version":"3.12.2","platform":"ruby","checksum":"8652db70b25ae3378b7274477a906b6ad1833a7b7cfbb001a03f49dd1c1d6a0d"}, +{"name":"rspec-mocks","version":"3.12.3","platform":"ruby","checksum":"cc0a1176707e641a2c66c71fe769486fec57d7df8ec7e34320f8957a1363026b"}, {"name":"rspec-parameterized","version":"0.5.0","platform":"ruby","checksum":"f163ac07b5edd1eeb13136480623db7020852c70cf0ad2fa98e31384ae162454"}, -{"name":"rspec-rails","version":"5.0.1","platform":"ruby","checksum":"c61e7f35db2266f83b3cc58a340fc3ec0bd6344818040430fd5ddc99775242de"}, +{"name":"rspec-rails","version":"6.0.1","platform":"ruby","checksum":"016c8ebd5b38ce5cbce949de2f5b28f2bde7bb78d4de26940516713597b26e34"}, {"name":"rspec-retry","version":"0.6.1","platform":"ruby","checksum":"86b7e8513c5b0c713c2e28854f4d996deb8efa6304eef50f0ad68ee6c563d8da"}, -{"name":"rspec-support","version":"3.10.3","platform":"ruby","checksum":"65c88f8cbe579461f411097682e6402960eae327eef08e86ef581b8c609e4c5e"}, +{"name":"rspec-support","version":"3.12.0","platform":"ruby","checksum":"dd4d44b247ff679b95b5607ac5641d197a5f9b1d33f916123cb98fc5f917c58b"}, {"name":"rspec_junit_formatter","version":"0.6.0","platform":"ruby","checksum":"40dde674e6ae4e6cc0ff560da25497677e34fefd2338cc467a8972f602b62b15"}, {"name":"rspec_profiling","version":"0.0.6","platform":"ruby","checksum":"7a45697f79dcec9a174a0e26703465f6bd52ee78e8d798741240bfcef38f6e6e"}, {"name":"rubocop","version":"1.38.0","platform":"ruby","checksum":"64a64a66d746bd417224c0292d08d8bf5affcfe8fbfc3d50a36810ee8c8a1eba"}, @@ -566,7 +566,15 @@ {"name":"sprite-factory","version":"1.7.1","platform":"ruby","checksum":"5586524a1aec003241f1abc6852b61433e988aba5ee2b55f906387bf49b01ba2"}, {"name":"sprockets","version":"3.7.2","platform":"ruby","checksum":"5ea1d7facd09203c1aa196afd6178208cd25abdbcc2a9978810a2f0754e152a0"}, {"name":"sprockets-rails","version":"3.4.2","platform":"ruby","checksum":"36d6327757ccf7460a00d1d52b2d5ef0019a4670503046a129fa1fb1300931ad"}, -{"name":"sqlite3","version":"1.4.2","platform":"ruby","checksum":"e8b8ef3b0f75c18e1a7ee62c5678c827e99389e53fa55eb7a9a5f57459004a52"}, +{"name":"sqlite3","version":"1.6.0","platform":"aarch64-linux","checksum":"360ac488c2e0f7569ee757c80e73941c30cf5d5be3e5e6af747d2d9c8058841b"}, +{"name":"sqlite3","version":"1.6.0","platform":"arm-linux","checksum":"8adbe1c4845832c8ff295d263adf880f6e045f7dd06eb1c179e45349a963eced"}, +{"name":"sqlite3","version":"1.6.0","platform":"arm64-darwin","checksum":"b691ab812651de0607aabd6005642c8f4611f773e324cbed66b4bfa0da864c59"}, +{"name":"sqlite3","version":"1.6.0","platform":"ruby","checksum":"3d74af62d3cbf51856c4f9acd17e350d1d58c7e9639dbd3d7a38b00acac54438"}, +{"name":"sqlite3","version":"1.6.0","platform":"x64-mingw-ucrt","checksum":"c4ccd5a01c2feeb7370d34887f97c0c62fe58d7830d81adf9977afd48425fa4d"}, +{"name":"sqlite3","version":"1.6.0","platform":"x64-mingw32","checksum":"43211334166269933fbd85b04f45be681521f36f7e779bd37f2d6005257d8360"}, +{"name":"sqlite3","version":"1.6.0","platform":"x86-linux","checksum":"639e3a9909897c03d5a014da7412506afb985a55007b86f6830c8a7cac65385a"}, +{"name":"sqlite3","version":"1.6.0","platform":"x86_64-darwin","checksum":"e177778f16415370eb1e7401a492ec25c48d00ac5ff83789ba905e50ee083c64"}, +{"name":"sqlite3","version":"1.6.0","platform":"x86_64-linux","checksum":"a2488dcf0e72928bab2b15b934113ce8d7a3b4031277e362d66e40956d5c709e"}, {"name":"ssh_data","version":"1.3.0","platform":"ruby","checksum":"ec7c1e95a3aebeee412147998f4c147b4b05da6ed0aafda6083f9449318eaac0"}, {"name":"ssrf_filter","version":"1.0.7","platform":"ruby","checksum":"80dc5728e5743201239e465c7e14290c7acf1de3b9f1f664579112429c79953a"}, {"name":"stackprof","version":"0.2.21","platform":"ruby","checksum":"2b6406c55dc2e134b2789c4cc631d96e67da87821a166f4ae12f15bec5cff5ae"}, diff --git a/Gemfile.lock b/Gemfile.lock index 6d15bde8646..7b651712899 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -297,7 +297,7 @@ GEM coderay (1.1.3) colored2 (3.1.2) commonmarker (0.23.6) - concurrent-ruby (1.1.10) + concurrent-ruby (1.2.0) connection_pool (2.3.0) cork (0.3.0) colored2 (~> 3.1) @@ -425,7 +425,7 @@ GEM launchy (~> 2.1) mail (~> 2.7) encryptor (3.0.0) - erubi (1.11.0) + erubi (1.12.0) escape_utils (1.2.1) et-orbi (1.2.7) tzinfo @@ -904,7 +904,7 @@ GEM mini_histogram (0.3.1) mini_magick (4.10.1) mini_mime (1.1.2) - mini_portile2 (2.8.0) + mini_portile2 (2.8.1) minitest (5.11.3) mixlib-cli (2.1.8) mixlib-config (3.0.9) @@ -1155,7 +1155,7 @@ GEM rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) - rails-html-sanitizer (1.4.4) + rails-html-sanitizer (1.5.0) loofah (~> 2.19, >= 2.19.1) rails-i18n (7.0.3) i18n (>= 0.7, < 2) @@ -1175,7 +1175,7 @@ GEM ffi (>= 1.0.6) msgpack (>= 0.4.3) optimist (>= 3.0.0) - rbtree (0.4.4) + rbtree (0.4.6) rchardet (1.8.0) rdoc (6.3.2) re2 (1.6.0) @@ -1222,40 +1222,40 @@ GEM chunky_png rqrcode-rails3 (0.1.7) rqrcode (>= 0.4.2) - rspec (3.10.0) - rspec-core (~> 3.10.0) - rspec-expectations (~> 3.10.0) - rspec-mocks (~> 3.10.0) + rspec (3.12.0) + rspec-core (~> 3.12.0) + rspec-expectations (~> 3.12.0) + rspec-mocks (~> 3.12.0) rspec-benchmark (0.6.0) benchmark-malloc (~> 0.2) benchmark-perf (~> 0.6) benchmark-trend (~> 0.4) rspec (>= 3.0) - rspec-core (3.10.2) - rspec-support (~> 3.10.0) - rspec-expectations (3.10.1) + rspec-core (3.12.0) + rspec-support (~> 3.12.0) + rspec-expectations (3.12.2) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.10.0) - rspec-mocks (3.10.3) + rspec-support (~> 3.12.0) + rspec-mocks (3.12.3) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.10.0) + rspec-support (~> 3.12.0) rspec-parameterized (0.5.0) binding_ninja (>= 0.2.3) parser proc_to_ast rspec (>= 2.13, < 4) unparser - rspec-rails (5.0.1) - actionpack (>= 5.2) - activesupport (>= 5.2) - railties (>= 5.2) - rspec-core (~> 3.10) - rspec-expectations (~> 3.10) - rspec-mocks (~> 3.10) - rspec-support (~> 3.10) + rspec-rails (6.0.1) + actionpack (>= 6.1) + activesupport (>= 6.1) + railties (>= 6.1) + rspec-core (~> 3.11) + rspec-expectations (~> 3.11) + rspec-mocks (~> 3.11) + rspec-support (~> 3.11) rspec-retry (0.6.1) rspec-core (> 3.3) - rspec-support (3.10.3) + rspec-support (3.12.0) rspec_junit_formatter (0.6.0) rspec-core (>= 2, < 4, != 2.12.0) rspec_profiling (0.0.6) @@ -1414,7 +1414,8 @@ GEM actionpack (>= 5.2) activesupport (>= 5.2) sprockets (>= 3.0.0) - sqlite3 (1.4.2) + sqlite3 (1.6.0) + mini_portile2 (~> 2.8.0) ssh_data (1.3.0) ssrf_filter (1.0.7) stackprof (0.2.21) @@ -1808,7 +1809,7 @@ DEPENDENCIES rqrcode-rails3 (~> 0.1.7) rspec-benchmark (~> 0.6.0) rspec-parameterized - rspec-rails (~> 5.0.1) + rspec-rails (~> 6.0.1) rspec-retry (~> 0.6.1) rspec_junit_formatter rspec_profiling (~> 0.0.6) diff --git a/app/assets/javascripts/pages/admin/application_settings/account_and_limits.js b/app/assets/javascripts/pages/admin/application_settings/account_and_limits.js index 455c637a6b3..c054171935d 100644 --- a/app/assets/javascripts/pages/admin/application_settings/account_and_limits.js +++ b/app/assets/javascripts/pages/admin/application_settings/account_and_limits.js @@ -20,10 +20,63 @@ function setUserInternalRegexPlaceholder(checkbox) { } } -export default function initUserInternalRegexPlaceholder() { +function initUserInternalRegexPlaceholder() { const checkbox = document.getElementById('application_setting_user_default_external'); setUserInternalRegexPlaceholder(checkbox); checkbox.addEventListener('change', () => { setUserInternalRegexPlaceholder(checkbox); }); } + +/** + * Sets up logic inside "Dormant users" subsection: + * - checkbox enables/disables additional input + * - shows/hides an inline error on input validation + */ +function initDeactivateDormantUsersPeriodInputSection() { + const DISPLAY_NONE_CLASS = 'gl-display-none'; + + /** @type {HTMLInputElement} */ + const checkbox = document.getElementById('application_setting_deactivate_dormant_users'); + /** @type {HTMLInputElement} */ + const input = document.getElementById('application_setting_deactivate_dormant_users_period'); + /** @type {HTMLDivElement} */ + const errorLabel = document.getElementById( + 'application_setting_deactivate_dormant_users_period_error', + ); + + const hideInputErrorLabel = () => { + if (input.checkValidity()) { + errorLabel.classList.add(DISPLAY_NONE_CLASS); + } + }; + + const handleInputInvalidState = (event) => { + event.preventDefault(); + event.stopImmediatePropagation(); + errorLabel.classList.remove(DISPLAY_NONE_CLASS); + return false; + }; + + const updateInputDisabledState = () => { + input.disabled = !checkbox.checked; + if (input.disabled) { + hideInputErrorLabel(); + } + }; + + // Show error when input is invalid + input.addEventListener('invalid', handleInputInvalidState); + // Hide error when input changes + input.addEventListener('input', hideInputErrorLabel); + input.addEventListener('change', hideInputErrorLabel); + + // Handle checkbox change and set initial state + checkbox.addEventListener('change', updateInputDisabledState); + updateInputDisabledState(); +} + +export default function initAccountAndLimitsSection() { + initUserInternalRegexPlaceholder(); + initDeactivateDormantUsersPeriodInputSection(); +} diff --git a/app/assets/javascripts/pages/admin/application_settings/general/index.js b/app/assets/javascripts/pages/admin/application_settings/general/index.js index c48d99da990..8a810ca649c 100644 --- a/app/assets/javascripts/pages/admin/application_settings/general/index.js +++ b/app/assets/javascripts/pages/admin/application_settings/general/index.js @@ -1,9 +1,9 @@ -import initUserInternalRegexPlaceholder from '../account_and_limits'; +import initAccountAndLimitsSection from '../account_and_limits'; import initGitpod from '../gitpod'; import initSignupRestrictions from '../signup_restrictions'; (() => { - initUserInternalRegexPlaceholder(); + initAccountAndLimitsSection(); initGitpod(); initSignupRestrictions(); })(); diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 3a48b113473..e60a54fa745 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -33,7 +33,7 @@ module ApplicationSettingImplementation DEFAULT_MINIMUM_PASSWORD_LENGTH = 8 class_methods do - def defaults + def defaults # rubocop:disable Metrics/AbcSize { admin_mode: false, after_sign_up_text: nil, @@ -249,7 +249,13 @@ module ApplicationSettingImplementation bulk_import_enabled: false, allow_runner_registration_token: true, user_defaults_to_private_profile: false - } + }.tap do |hsh| + hsh.merge!(non_production_defaults) unless Rails.env.production? + end + end + + def non_production_defaults + {} end def default_commit_email_hostname diff --git a/app/models/performance_monitoring/prometheus_dashboard.rb b/app/models/performance_monitoring/prometheus_dashboard.rb index 37bf080ae49..6fea3abf3d9 100644 --- a/app/models/performance_monitoring/prometheus_dashboard.rb +++ b/app/models/performance_monitoring/prometheus_dashboard.rb @@ -58,7 +58,7 @@ module PerformanceMonitoring rescue Gitlab::Metrics::Dashboard::Errors::LayoutError => e [e.message] rescue ActiveModel::ValidationError => e - e.model.errors.map { |attr, error| "#{attr}: #{error}" } + e.model.errors.map { |error| "#{error.attribute}: #{error.message}" } end private diff --git a/app/services/snippets/count_service.rb b/app/services/snippets/count_service.rb index 9a3d33c75cf..ba421c5777e 100644 --- a/app/services/snippets/count_service.rb +++ b/app/services/snippets/count_service.rb @@ -70,7 +70,7 @@ module Snippets count(case when visibility_level=#{Snippet::PUBLIC} OR visibility_level=#{Snippet::INTERNAL} then 1 else null end) as are_public_or_internal, count(*) as total ") - .first + .take end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/views/admin/application_settings/_account_and_limit.html.haml b/app/views/admin/application_settings/_account_and_limit.html.haml index 21f69f6700f..87c251aa10c 100644 --- a/app/views/admin/application_settings/_account_and_limit.html.haml +++ b/app/views/admin/application_settings/_account_and_limit.html.haml @@ -55,7 +55,9 @@ = f.gitlab_ui_checkbox_component :deactivate_dormant_users, _('Deactivate dormant users after a period of inactivity'), help_text: _('Users can reactivate their account by signing in. %{link_start}Learn more.%{link_end}').html_safe % { link_start: dormant_users_help_link_start, link_end: '</a>'.html_safe } .form-group = f.label :deactivate_dormant_users_period, _('Days of inactivity before deactivation'), class: 'label-light' - = f.number_field :deactivate_dormant_users_period, class: 'form-control gl-form-input', min: '90', step: '1' + = f.number_field :deactivate_dormant_users_period, class: 'form-control gl-form-input', min: 90, required: true + #application_setting_deactivate_dormant_users_period_error.form-text.gl-text-red-500.gl-display-none + = _('Please enter a value of 90 days or more') .form-text.text-muted = _('Must be 90 days or more.') diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 93237e1e71e..80c4b3ac016 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -20651,7 +20651,6 @@ Represents a vulnerability. | <a id="vulnerabilityscanner"></a>`scanner` | [`VulnerabilityScanner`](#vulnerabilityscanner) | Scanner metadata for the vulnerability. | | <a id="vulnerabilityseverity"></a>`severity` | [`VulnerabilitySeverity`](#vulnerabilityseverity) | Severity of the vulnerability (INFO, UNKNOWN, LOW, MEDIUM, HIGH, CRITICAL). | | <a id="vulnerabilitystate"></a>`state` | [`VulnerabilityState`](#vulnerabilitystate) | State of the vulnerability (DETECTED, CONFIRMED, RESOLVED, DISMISSED). | -| <a id="vulnerabilitystatecomment"></a>`stateComment` | [`String`](#string) | Comment given for the vulnerability state change. | | <a id="vulnerabilitytitle"></a>`title` | [`String`](#string) | Title of the vulnerability. | | <a id="vulnerabilityupdatedat"></a>`updatedAt` | [`Time`](#time) | Timestamp of when the vulnerability was last updated. | | <a id="vulnerabilityusernotescount"></a>`userNotesCount` | [`Int!`](#int) | Number of user notes attached to the vulnerability. | diff --git a/doc/api/product_analytics.md b/doc/api/product_analytics.md index 1856dd07b4a..d9b84995edb 100644 --- a/doc/api/product_analytics.md +++ b/doc/api/product_analytics.md @@ -25,9 +25,10 @@ POST /projects/:id/product_analytics/request/load POST /projects/:id/product_analytics/request/dry-run ``` -| Attribute | Type | Required | Description | -| --------- |------------------| -------- |---------------------------------------------------------------| -| `id` | integer | yes | The ID of a project that the current user has read access to. | +| Attribute | Type | Required | Description | +|-----------------|------------------| -------- |---------------------------------------------------------------------------------------------| +| `id` | integer | yes | The ID of a project that the current user has read access to. | +| `include_token` | boolean | no | Whether to include the access token in the response. (Only required for funnel generation.) | ### Request body diff --git a/doc/development/database/table_partitioning.md b/doc/development/database/table_partitioning.md index fe92ebc768a..0d5e3c233f6 100644 --- a/doc/development/database/table_partitioning.md +++ b/doc/development/database/table_partitioning.md @@ -381,6 +381,12 @@ result in a `Key is still referenced from table ...` error and updating the partition column on the source table would raise a `Key is not present in table ...` error. +This migration can be automatically generated using: + +```shell +./scripts/partitioning/generate-fk --source source_table_name --target target_table_name +``` + ### Step 5 - Swap primary key Swap the primary key including the partitioning key column. This can be done only after diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index 6e1227302a6..025d35ed9d6 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -151,6 +151,7 @@ Vulnerabilities that have been detected and are false positives are flagged as f False positive detection is available in a subset of the [supported languages](#supported-languages-and-frameworks) and [analyzers](analyzers.md): - Ruby, in the Brakeman-based analyzer +- Go ![SAST false-positives show in Vulnerability Pages](img/sast_vulnerability_page_fp_detection_v15_2.png) diff --git a/doc/user/project/import/perforce.md b/doc/user/project/import/perforce.md index ca50a32836e..a96297b1a38 100644 --- a/doc/user/project/import/perforce.md +++ b/doc/user/project/import/perforce.md @@ -16,7 +16,7 @@ The following list illustrates the main differences between Perforce Helix and Git: - In general, the biggest difference is that Perforce branching is heavyweight - compared to Git's lightweight branching. When you create a branch in Perforce, + compared to Git lightweight branching. When you create a branch in Perforce, it creates an integration record in their proprietary database for every file in the branch, regardless how many were actually changed. With Git, however, a single SHA acts as a pointer to the state of the whole repository after the @@ -56,7 +56,7 @@ Here's a few links to get you started: - [`git-p4` example usage](https://git.wiki.kernel.org/index.php/Git-p4_Usage) - [Git book migration guide](https://git-scm.com/book/en/v2/Git-and-Other-Systems-Migrating-to-Git#_perforce_import) -Note that `git p4` and `git filter-branch` are not very good at +`git p4` and `git filter-branch` are not very good at creating small and efficient Git pack files. So it might be a good idea to spend time and CPU to properly repack your repository before sending it for the first time to your GitLab server. See diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 2adcdb0598a..197bd420295 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -504,12 +504,12 @@ module API def render_validation_error!(model, status = 400) if model.errors.any? - render_api_error!(model_error_messages(model) || '400 Bad Request', status) + render_api_error!(model_errors(model).messages || '400 Bad Request', status) end end - def model_error_messages(model) - model.errors.messages + def model_errors(model) + model.errors end def render_api_error_with_reason!(status, message, reason) diff --git a/lib/api/helpers/users_helpers.rb b/lib/api/helpers/users_helpers.rb index e80b89488a2..f97071d9a97 100644 --- a/lib/api/helpers/users_helpers.rb +++ b/lib/api/helpers/users_helpers.rb @@ -12,10 +12,14 @@ module API params :optional_index_params_ee do end - def model_error_messages(model) - super.tap do |error_messages| + def model_errors(model) + super.tap do |errors| # Remapping errors from nested associations. - error_messages[:bio] = error_messages.delete(:"user_detail.bio") if error_messages.has_key?(:"user_detail.bio") + next unless errors.has_key?(:"user_detail.bio") + + errors.delete(:"user_detail.bio").each do |message| + errors.add(:bio, message) + end end end diff --git a/lib/feature.rb b/lib/feature.rb index d012639d489..892d32c9b73 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -206,6 +206,7 @@ module Feature # to register Flipper groups. # See https://docs.gitlab.com/ee/development/feature_flags/index.html def register_feature_groups + Flipper.register(:gitlab_team_members) { |actor| FeatureGroups::GitlabTeamMembers.enabled?(actor.thing) } end def register_definitions diff --git a/lib/feature_groups/gitlab_team_members.rb b/lib/feature_groups/gitlab_team_members.rb new file mode 100644 index 00000000000..7f4c597fddd --- /dev/null +++ b/lib/feature_groups/gitlab_team_members.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module FeatureGroups + class GitlabTeamMembers + GITLAB_COM_GROUP_ID = 6543 + + class << self + def enabled?(thing) + return false unless Gitlab.com? + + team_member?(thing) + end + + private + + def team_member?(thing) + thing.is_a?(::User) && gitlab_com_member_ids.include?(thing.id) + end + + def gitlab_com + @gitlab_com ||= ::Group.find(GITLAB_COM_GROUP_ID) + end + + def gitlab_com_member_ids + Rails.cache.fetch("gitlab_team_members", expires_in: 1.hour) do + gitlab_com.members.pluck_user_ids.to_set + end + end + end + end +end diff --git a/lib/generators/gitlab/partitioning/foreign_keys_generator.rb b/lib/generators/gitlab/partitioning/foreign_keys_generator.rb new file mode 100644 index 00000000000..e013188c753 --- /dev/null +++ b/lib/generators/gitlab/partitioning/foreign_keys_generator.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +require 'rails/generators' +require 'rails/generators/active_record/migration' + +module Gitlab + module Partitioning + class ForeignKeysGenerator < Rails::Generators::Base + include ActiveRecord::Generators::Migration + + desc 'This generator creates the migrations needed for updating the foreign keys when partitioning the tables' + + source_root File.expand_path('templates', __dir__) + + class_option :target, type: :string, required: true, desc: 'Target table name' + class_option :source, type: :string, required: true, desc: 'Source table name' + class_option :partitioning_column, type: :string, default: :partition_id, + desc: 'The column that is used for partitioning' + class_option :database, type: :string, default: :ci, + desc: 'Database name connection' + + def create_fk_index_migration + migration_template( + '../templates/foreign_key_index.rb.template', + fk_index_file_name) + end + + def create_fk_definition_migration + migration_template( + '../templates/foreign_key_definition.rb.template', + fk_definition_file_name) + end + + def create_fk_validation_migration + migration_template( + '../templates/foreign_key_validation.rb.template', + fk_validation_file_name) + end + + def remove_old_fk_migration + migration_template( + '../templates/foreign_key_removal.rb.template', + fk_removal_file_name) + end + + private + + def fk_index_file_name + post_migration_file_path( + "add_fk_index_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_definition_file_name + post_migration_file_path( + "add_fk_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_validation_file_name + post_migration_file_path( + "validate_fk_on_#{source_table_name}_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_removal_file_name + post_migration_file_path( + "remove_fk_to_#{target_table_name}_#{source_table_name}_on_#{foreign_key_column}.rb") + end + + def post_migration_file_path(name) + File.join(db_migrate_path, name) + end + + def db_migrate_path + super.sub('migrate', 'post_migrate') + end + + def source_table_name + options[:source] + end + + def target_table_name + options[:target] + end + + def partitioning_column + options[:partitioning_column] + end + + def foreign_keys_candidates + connection + .foreign_keys(source_table_name) + .select { |fk| fk.to_table == target_table_name } + .reject { |fk| fk.name.end_with?('_p') } + end + + def fk_candidate + @fk_candidate ||= select_foreign_key + end + + def foreign_key_name + fk_candidate.name + end + + def partitioned_foreign_key_name + "#{foreign_key_name}_p" + end + + def foreign_key_column + fk_candidate.column + end + + def fk_on_delete_option + fk_candidate.on_delete + end + + def fk_target_column + fk_candidate.primary_key + end + + def connection + Gitlab::Database + .database_base_models + .fetch(options[:database]) + .connection + end + + def select_foreign_key + case foreign_keys_candidates.size + when 0 + raise Thor::InvocationError, "No FK found between #{source_table_name} and #{target_table_name}" + when 1 + foreign_keys_candidates.first + else + select_fk_from_user_input + end + end + + def select_fk_from_user_input + options = (0...foreign_keys_candidates.size).to_a.map(&:to_s) + + say "There are multiple FKs between #{source_table_name} and #{target_table_name}:" + foreign_keys_candidates.each.with_index do |fk, index| + say "\t#{index} : #{fk}" + end + + input = ask("Please select one:", limited_to: options, default: '0') + + foreign_keys_candidates.fetch(input.to_i) + end + end + end +end diff --git a/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template new file mode 100644 index 00000000000..f2f9acea923 --- /dev/null +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :<%= source_table_name %> + TARGET_TABLE_NAME = :<%= target_table_name %> + COLUMN = :<%= foreign_key_column %> + TARGET_COLUMN = :<%= fk_target_column %> + FK_NAME = :<%= partitioned_foreign_key_name %> + PARTITION_COLUMN = :<%= partitioning_column %> + + def up + add_concurrent_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: [PARTITION_COLUMN, COLUMN], + target_column: [PARTITION_COLUMN, TARGET_COLUMN], + validate: false, + reverse_lock_order: true, + on_update: :cascade, + on_delete: :<%= fk_on_delete_option %>, + name: FK_NAME + ) + end + + def down + with_lock_retries do + remove_foreign_key_if_exists( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + name: FK_NAME, + reverse_lock_order: true + ) + end + end +end diff --git a/lib/generators/gitlab/partitioning/templates/foreign_key_index.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_index.rb.template new file mode 100644 index 00000000000..4896d931333 --- /dev/null +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_index.rb.template @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + INDEX_NAME = :index_<%= source_table_name -%>_on_<%= partitioning_column -%>_<%= foreign_key_column %> + TABLE_NAME = :<%= source_table_name %> + COLUMNS = [:<%= partitioning_column -%>, :<%= foreign_key_column -%>] + + def up + add_concurrent_index(TABLE_NAME, COLUMNS, name: INDEX_NAME) + end + + def down + remove_concurrent_index_by_name(TABLE_NAME, INDEX_NAME) + end +end diff --git a/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template new file mode 100644 index 00000000000..b4e881074ad --- /dev/null +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :<%= source_table_name %> + TARGET_TABLE_NAME = :<%= target_table_name %> + COLUMN = :<%= foreign_key_column %> + TARGET_COLUMN = :<%= fk_target_column %> + FK_NAME = :<%= foreign_key_name %> + + def up + with_lock_retries do + remove_foreign_key_if_exists( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + name: FK_NAME, + reverse_lock_order: true + ) + end + end + + def down + add_concurrent_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: COLUMN, + target_column: TARGET_COLUMN, + validate: true, + reverse_lock_order: true, + on_delete: :<%= fk_on_delete_option %>, + name: FK_NAME + ) + end +end diff --git a/lib/generators/gitlab/partitioning/templates/foreign_key_validation.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_validation.rb.template new file mode 100644 index 00000000000..bad7d17a51b --- /dev/null +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_validation.rb.template @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + TABLE_NAME = :<%= source_table_name %> + FK_NAME = :<%= partitioned_foreign_key_name %> + COLUMNS = [:<%= partitioning_column -%>, :<%= foreign_key_column -%>] + + def up + validate_foreign_key(TABLE_NAME, COLUMNS, name: FK_NAME) + end + + def down + # no-op + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 40735f8d62a..99acbce98f7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -31535,6 +31535,9 @@ msgstr "" msgid "Please enter a valid time interval" msgstr "" +msgid "Please enter a value of 90 days or more" +msgstr "" + msgid "Please enter your current password." msgstr "" diff --git a/scripts/partitioning/generate-fk b/scripts/partitioning/generate-fk new file mode 100755 index 00000000000..4f2dac1d61e --- /dev/null +++ b/scripts/partitioning/generate-fk @@ -0,0 +1,3 @@ +#!/bin/bash + +exec bundle exec rails generate gitlab:partitioning:foreign_keys "$@" diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index f0482d8c511..d8929e1edfb 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -167,7 +167,7 @@ RSpec.describe 'Admin updates settings', feature_category: :not_owned do context 'when not Gitlab.com' do let(:dot_com?) { false } - it 'changes Dormant users' do + it 'changes dormant users' do expect(page).to have_unchecked_field('Deactivate dormant users after a period of inactivity') expect(current_settings.deactivate_dormant_users).to be_falsey @@ -184,7 +184,7 @@ RSpec.describe 'Admin updates settings', feature_category: :not_owned do expect(page).to have_checked_field('Deactivate dormant users after a period of inactivity') end - it 'change Dormant users period' do + it 'change dormant users period' do expect(page).to have_field _('Days of inactivity before deactivation') page.within(find('[data-testid="account-limit"]')) do @@ -198,6 +198,27 @@ RSpec.describe 'Admin updates settings', feature_category: :not_owned do expect(page).to have_field _('Days of inactivity before deactivation'), with: '90' end + + it 'displays dormant users period field validation error', :js do + selector = '#application_setting_deactivate_dormant_users_period_error' + expect(page).not_to have_selector(selector, visible: :visible) + + page.within(find('[data-testid="account-limit"]')) do + check 'application_setting_deactivate_dormant_users' + fill_in _('application_setting_deactivate_dormant_users_period'), with: '30' + click_button 'Save changes' + end + + expect(page).to have_selector(selector, visible: :visible) + end + + it 'auto disables dormant users period field depending on parent checkbox', :js do + uncheck 'application_setting_deactivate_dormant_users' + expect(page).to have_field('application_setting_deactivate_dormant_users_period', disabled: true) + + check 'application_setting_deactivate_dormant_users' + expect(page).to have_field('application_setting_deactivate_dormant_users_period', disabled: false) + end end end diff --git a/spec/features/projects/issues/design_management/user_views_designs_with_svg_xss_spec.rb b/spec/features/projects/issues/design_management/user_views_designs_with_svg_xss_spec.rb index a45b9b718c3..bbc54382ae6 100644 --- a/spec/features/projects/issues/design_management/user_views_designs_with_svg_xss_spec.rb +++ b/spec/features/projects/issues/design_management/user_views_designs_with_svg_xss_spec.rb @@ -42,7 +42,7 @@ RSpec.describe 'User views an SVG design that contains XSS', :js, feature_catego } # With the page loaded, there should be no alert modal - expect(run_expectation).to raise_error( + expect { run_expectation.call }.to raise_error( Capybara::ModalNotFound, 'Unable to find modal dialog' ) @@ -51,6 +51,6 @@ RSpec.describe 'User views an SVG design that contains XSS', :js, feature_catego # With an alert modal displaying, the modal should be dismissable. execute_script('alert(true)') - expect(run_expectation).not_to raise_error + expect { run_expectation.call }.not_to raise_error end end diff --git a/spec/frontend/pages/admin/application_settings/account_and_limits_spec.js b/spec/frontend/pages/admin/application_settings/account_and_limits_spec.js index 85ed94b748d..d422f5dade3 100644 --- a/spec/frontend/pages/admin/application_settings/account_and_limits_spec.js +++ b/spec/frontend/pages/admin/application_settings/account_and_limits_spec.js @@ -1,20 +1,15 @@ -import $ from 'jquery'; import { loadHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; -import initUserInternalRegexPlaceholder, { +import initAccountAndLimitsSection, { PLACEHOLDER_USER_EXTERNAL_DEFAULT_FALSE, PLACEHOLDER_USER_EXTERNAL_DEFAULT_TRUE, } from '~/pages/admin/application_settings/account_and_limits'; describe('AccountAndLimits', () => { const FIXTURE = 'application_settings/accounts_and_limit.html'; - let $userDefaultExternal; - let $userInternalRegex; beforeEach(() => { loadHTMLFixture(FIXTURE); - initUserInternalRegexPlaceholder(); - $userDefaultExternal = $('#application_setting_user_default_external'); - $userInternalRegex = document.querySelector('#application_setting_user_default_internal_regex'); + initAccountAndLimitsSection(); }); afterEach(() => { @@ -22,18 +17,62 @@ describe('AccountAndLimits', () => { }); describe('Changing of userInternalRegex when userDefaultExternal', () => { + /** @type {HTMLInputElement} */ + let userDefaultExternalCheckbox; + /** @type {HTMLInputElement} */ + let userInternalRegexInput; + + beforeEach(() => { + userDefaultExternalCheckbox = document.getElementById( + 'application_setting_user_default_external', + ); + userInternalRegexInput = document.getElementById( + 'application_setting_user_default_internal_regex', + ); + }); + it('is unchecked', () => { - expect($userDefaultExternal.prop('checked')).toBe(false); - expect($userInternalRegex.placeholder).toEqual(PLACEHOLDER_USER_EXTERNAL_DEFAULT_FALSE); - expect($userInternalRegex.readOnly).toBe(true); + expect(userDefaultExternalCheckbox.checked).toBe(false); + expect(userInternalRegexInput.placeholder).toEqual(PLACEHOLDER_USER_EXTERNAL_DEFAULT_FALSE); + expect(userInternalRegexInput.readOnly).toBe(true); }); it('is checked', () => { - if (!$userDefaultExternal.prop('checked')) $userDefaultExternal.click(); + if (!userDefaultExternalCheckbox.checked) userDefaultExternalCheckbox.click(); + + expect(userDefaultExternalCheckbox.checked).toBe(true); + expect(userInternalRegexInput.placeholder).toEqual(PLACEHOLDER_USER_EXTERNAL_DEFAULT_TRUE); + expect(userInternalRegexInput.readOnly).toBe(false); + }); + }); + + describe('Dormant users period input logic', () => { + /** @type {HTMLInputElement} */ + let checkbox; + /** @type {HTMLInputElement} */ + let input; + + const updateCheckbox = (checked) => { + checkbox.checked = checked; + checkbox.dispatchEvent(new Event('change')); + }; + + beforeEach(() => { + checkbox = document.getElementById('application_setting_deactivate_dormant_users'); + input = document.getElementById('application_setting_deactivate_dormant_users_period'); + }); + + it('initial state', () => { + expect(checkbox.checked).toBe(false); + expect(input.disabled).toBe(true); + }); + + it('changes field enabled flag on checkbox change', () => { + updateCheckbox(true); + expect(input.disabled).toBe(false); - expect($userDefaultExternal.prop('checked')).toBe(true); - expect($userInternalRegex.placeholder).toEqual(PLACEHOLDER_USER_EXTERNAL_DEFAULT_TRUE); - expect($userInternalRegex.readOnly).toBe(false); + updateCheckbox(false); + expect(input.disabled).toBe(true); }); }); }); diff --git a/spec/lib/feature_groups/gitlab_team_members_spec.rb b/spec/lib/feature_groups/gitlab_team_members_spec.rb new file mode 100644 index 00000000000..103cb62af1c --- /dev/null +++ b/spec/lib/feature_groups/gitlab_team_members_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe FeatureGroups::GitlabTeamMembers do # rubocop:disable RSpec/MissingFeatureCategory + let_it_be(:gitlab_com) { create(:group) } + let_it_be_with_reload(:member) { create(:user).tap { |user| gitlab_com.add_developer(user) } } + let_it_be_with_reload(:non_member) { create(:user) } + + before do + stub_const("#{described_class.name}::GITLAB_COM_GROUP_ID", gitlab_com.id) + end + + describe '#enabled?' do + context 'when not on gitlab.com' do + before do + allow(Gitlab).to receive(:com?).and_return(false) + end + + it 'returns false' do + expect(described_class.enabled?(member)).to eq(false) + end + end + + context 'when on gitlab.com' do + before do + allow(Gitlab).to receive(:com?).and_return(true) + end + + it 'returns true for gitlab-com group members' do + expect(described_class.enabled?(member)).to eq(true) + end + + it 'returns false for users not in gitlab-com' do + expect(described_class.enabled?(non_member)).to eq(false) + end + + it 'returns false when actor is not a user' do + expect(described_class.enabled?(gitlab_com)).to eq(false) + end + + it 'reloads members after 1 hour' do + expect(described_class.enabled?(non_member)).to eq(false) + + gitlab_com.add_developer(non_member) + + travel_to(2.hours.from_now) do + expect(described_class.enabled?(non_member)).to eq(true) + end + end + + it 'does not make queries on subsequent calls', :use_clean_rails_memory_store_caching do + described_class.enabled?(member) + non_member + + queries = ActiveRecord::QueryRecorder.new do + described_class.enabled?(member) + described_class.enabled?(non_member) + end + + expect(queries.count).to eq(0) + end + end + end +end diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index c087931d36a..82501cbd1aa 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Feature, stub_feature_flags: false do +RSpec.describe Feature, stub_feature_flags: false do # rubocop:disable RSpec/MissingFeatureCategory include StubVersion before do @@ -154,6 +154,17 @@ RSpec.describe Feature, stub_feature_flags: false do end end + describe '.register_feature_groups' do + before do + Flipper.unregister_groups + described_class.register_feature_groups + end + + it 'registers expected groups' do + expect(Flipper.groups).to include(an_object_having_attributes(name: :gitlab_team_members)) + end + end + describe '.enabled?' do before do allow(Feature).to receive(:log_feature_flag_states?).and_return(false) @@ -350,6 +361,22 @@ RSpec.describe Feature, stub_feature_flags: false do end end + context 'with gitlab_team_members feature group' do + let(:actor) { build_stubbed(:user) } + + before do + Flipper.unregister_groups + described_class.register_feature_groups + described_class.enable(:enabled_feature_flag, :gitlab_team_members) + end + + it 'delegates check to FeatureGroups::GitlabTeamMembers' do + expect(FeatureGroups::GitlabTeamMembers).to receive(:enabled?).with(actor) + + described_class.enabled?(:enabled_feature_flag, actor) + end + end + context 'with an individual actor' do let(:actor) { stub_feature_flag_gate('CustomActor:5') } let(:another_actor) { stub_feature_flag_gate('CustomActor:10') } diff --git a/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb new file mode 100644 index 00000000000..7c7ca8207ff --- /dev/null +++ b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'active_support/testing/stream' + +RSpec.describe Gitlab::Partitioning::ForeignKeysGenerator, :migration, :silence_stdout, +feature_category: :continuous_integration do + include ActiveSupport::Testing::Stream + include MigrationsHelpers + + before do + ActiveRecord::Schema.define do + create_table :_test_tmp_builds, force: :cascade do |t| + t.integer :partition_id + t.index [:id, :partition_id], unique: true + end + + create_table :_test_tmp_metadata, force: :cascade do |t| + t.integer :partition_id + t.references :builds, foreign_key: { to_table: :_test_tmp_builds, on_delete: :cascade } + end + end + end + + after do + FileUtils.rm_rf(destination_root) + + table(:schema_migrations).where(version: migrations.map(&:version)).delete_all + + active_record_base.connection.execute(<<~SQL) + DROP TABLE _test_tmp_metadata; + DROP TABLE _test_tmp_builds; + SQL + end + + let_it_be(:destination_root) { File.expand_path("../tmp", __dir__) } + + let(:generator_config) { { destination_root: destination_root } } + let(:generator_args) { ['--source', '_test_tmp_metadata', '--target', '_test_tmp_builds', '--database', 'main'] } + + context 'without foreign keys' do + let(:generator_args) { ['--source', '_test_tmp_metadata', '--target', 'projects', '--database', 'main'] } + + it 'does not generate migrations' do + output = capture(:stderr) { run_generator } + + expect(migrations).to be_empty + expect(output).to match(/No FK found between _test_tmp_metadata and projects/) + end + end + + context 'with one FK' do + it 'generates foreign key migrations' do + run_generator + + expect(migrations.sort_by(&:version).map(&:name)).to eq(%w[ + AddFkIndexToTestTmpMetadataOnPartitionIdAndBuildsId + AddFkToTestTmpMetadataOnPartitionIdAndBuildsId + ValidateFkOnTestTmpMetadataPartitionIdAndBuildsId + RemoveFkToTestTmpBuildsTestTmpMetadataOnBuildsId + ]) + + schema_migrate_up! + + fks = Gitlab::Database::PostgresForeignKey + .by_referenced_table_identifier('public._test_tmp_builds') + .by_constrained_table_identifier('public._test_tmp_metadata') + + expect(fks.size).to eq(1) + + foreign_key = fks.first + + expect(foreign_key.name).to end_with('_p') + expect(foreign_key.constrained_columns).to eq(%w[partition_id builds_id]) + expect(foreign_key.referenced_columns).to eq(%w[partition_id id]) + expect(foreign_key.on_delete_action).to eq('cascade') + expect(foreign_key.on_update_action).to eq('cascade') + + index = active_record_base.connection.indexes('_test_tmp_metadata').find do |index| + index.columns == %w[partition_id builds_id] + end + + expect(index).to be_present + end + end + + context 'with many FKs' do + before do + ActiveRecord::Schema.define do + add_reference :_test_tmp_metadata, :job, + foreign_key: { to_table: :_test_tmp_builds, on_delete: :cascade } + end + end + + it 'generates migrations for the selected FK' do + expect(Thor::LineEditor) + .to receive(:readline) + .with('Please select one: [0, 1] (0) ', { default: '0', limited_to: %w[0 1] }) + .and_return('1') + + run_generator + + expect(migrations.sort_by(&:version).map(&:name)).to eq(%w[ + AddFkIndexToTestTmpMetadataOnPartitionIdAndJobId + AddFkToTestTmpMetadataOnPartitionIdAndJobId + ValidateFkOnTestTmpMetadataPartitionIdAndJobId + RemoveFkToTestTmpBuildsTestTmpMetadataOnJobId + ]) + end + end + + def run_generator(args = generator_args, config = generator_config) + described_class.start(args, config) + end + + # We want to execute only the newly generated migrations + def migrations_paths + [File.join(destination_root, 'db', 'post_migrate')] + end + + # There is no need to migrate down before executing the tests because these + # migrations were not already executed and we don't need to run it after + # the tests because we're removing the tables. + def schema_migrate_down! + # no-op + end +end diff --git a/spec/lib/gitlab/database/batch_count_spec.rb b/spec/lib/gitlab/database/batch_count_spec.rb index 852cc719d01..53f8fe3dcd2 100644 --- a/spec/lib/gitlab/database/batch_count_spec.rb +++ b/spec/lib/gitlab/database/batch_count_spec.rb @@ -58,10 +58,10 @@ RSpec.describe Gitlab::Database::BatchCount do it 'reduces batch size by half and retry fetch' do too_big_batch_relation_mock = instance_double(ActiveRecord::Relation) allow(model).to receive_message_chain(:select, public_send: relation) - allow(relation).to receive(:where).with("id" => 0..calculate_batch_size(batch_size)).and_return(too_big_batch_relation_mock) + allow(relation).to receive(:where).with({ "id" => 0..calculate_batch_size(batch_size) }).and_return(too_big_batch_relation_mock) allow(too_big_batch_relation_mock).to receive(:send).and_raise(ActiveRecord::QueryCanceled) - expect(relation).to receive(:where).with("id" => 0..calculate_batch_size(batch_size / 2)).and_return(double(send: 1)) + expect(relation).to receive(:where).with({ "id" => 0..calculate_batch_size(batch_size / 2) }).and_return(double(send: 1)) subject.call(model, column, batch_size: batch_size, start: 0) end @@ -146,7 +146,7 @@ RSpec.describe Gitlab::Database::BatchCount do allow(model).to receive_message_chain(:select, public_send: relation) batch_end_id = min_id + calculate_batch_size(Gitlab::Database::BatchCounter::DEFAULT_BATCH_SIZE) - expect(relation).to receive(:where).with("id" => min_id..batch_end_id).and_return(double(send: 1)) + expect(relation).to receive(:where).with({ "id" => min_id..batch_end_id }).and_return(double(send: 1)) described_class.batch_count(model) end @@ -382,7 +382,7 @@ RSpec.describe Gitlab::Database::BatchCount do allow(model).to receive_message_chain(:select, public_send: relation) batch_end_id = min_id + calculate_batch_size(Gitlab::Database::BatchCounter::DEFAULT_DISTINCT_BATCH_SIZE) - expect(relation).to receive(:where).with("id" => min_id..batch_end_id).and_return(double(send: 1)) + expect(relation).to receive(:where).with({ "id" => min_id..batch_end_id }).and_return(double(send: 1)) described_class.batch_distinct_count(model) end @@ -468,7 +468,7 @@ RSpec.describe Gitlab::Database::BatchCount do allow(model).to receive_message_chain(:select, public_send: relation) batch_end_id = min_id + calculate_batch_size(Gitlab::Database::BatchCounter::DEFAULT_SUM_BATCH_SIZE) - expect(relation).to receive(:where).with("id" => min_id..batch_end_id).and_return(double(send: 1)) + expect(relation).to receive(:where).with({ "id" => min_id..batch_end_id }).and_return(double(send: 1)) described_class.batch_sum(model, column) end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 12fa115cc4e..9df23776be8 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1047,59 +1047,63 @@ RSpec.describe Gitlab::Database::MigrationHelpers do end describe '#foreign_key_exists?' do + let(:referenced_table_name) { '_test_gitlab_main_referenced' } + let(:referencing_table_name) { '_test_gitlab_main_referencing' } + before do model.connection.execute(<<~SQL) - create table referenced ( + create table #{referenced_table_name} ( id bigserial primary key not null ); - create table referencing ( + create table #{referencing_table_name} ( id bigserial primary key not null, non_standard_id bigint not null, - constraint fk_referenced foreign key (non_standard_id) references referenced(id) on delete cascade + constraint fk_referenced foreign key (non_standard_id) + references #{referenced_table_name}(id) on delete cascade ); SQL end shared_examples_for 'foreign key checks' do it 'finds existing foreign keys by column' do - expect(model.foreign_key_exists?(:referencing, target_table, column: :non_standard_id)).to be_truthy + expect(model.foreign_key_exists?(referencing_table_name, target_table, column: :non_standard_id)).to be_truthy end it 'finds existing foreign keys by name' do - expect(model.foreign_key_exists?(:referencing, target_table, name: :fk_referenced)).to be_truthy + expect(model.foreign_key_exists?(referencing_table_name, target_table, name: :fk_referenced)).to be_truthy end it 'finds existing foreign_keys by name and column' do - expect(model.foreign_key_exists?(:referencing, target_table, name: :fk_referenced, column: :non_standard_id)).to be_truthy + expect(model.foreign_key_exists?(referencing_table_name, target_table, name: :fk_referenced, column: :non_standard_id)).to be_truthy end it 'finds existing foreign_keys by name, column and on_delete' do - expect(model.foreign_key_exists?(:referencing, target_table, name: :fk_referenced, column: :non_standard_id, on_delete: :cascade)).to be_truthy + expect(model.foreign_key_exists?(referencing_table_name, target_table, name: :fk_referenced, column: :non_standard_id, on_delete: :cascade)).to be_truthy end it 'finds existing foreign keys by target table only' do - expect(model.foreign_key_exists?(:referencing, target_table)).to be_truthy + expect(model.foreign_key_exists?(referencing_table_name, target_table)).to be_truthy end it 'compares by column name if given' do - expect(model.foreign_key_exists?(:referencing, target_table, column: :user_id)).to be_falsey + expect(model.foreign_key_exists?(referencing_table_name, target_table, column: :user_id)).to be_falsey end it 'compares by target column name if given' do - expect(model.foreign_key_exists?(:referencing, target_table, primary_key: :user_id)).to be_falsey - expect(model.foreign_key_exists?(:referencing, target_table, primary_key: :id)).to be_truthy + expect(model.foreign_key_exists?(referencing_table_name, target_table, primary_key: :user_id)).to be_falsey + expect(model.foreign_key_exists?(referencing_table_name, target_table, primary_key: :id)).to be_truthy end it 'compares by foreign key name if given' do - expect(model.foreign_key_exists?(:referencing, target_table, name: :non_existent_foreign_key_name)).to be_falsey + expect(model.foreign_key_exists?(referencing_table_name, target_table, name: :non_existent_foreign_key_name)).to be_falsey end it 'compares by foreign key name and column if given' do - expect(model.foreign_key_exists?(:referencing, target_table, name: :non_existent_foreign_key_name, column: :non_standard_id)).to be_falsey + expect(model.foreign_key_exists?(referencing_table_name, target_table, name: :non_existent_foreign_key_name, column: :non_standard_id)).to be_falsey end it 'compares by foreign key name, column and on_delete if given' do - expect(model.foreign_key_exists?(:referencing, target_table, name: :fk_referenced, column: :non_standard_id, on_delete: :nullify)).to be_falsey + expect(model.foreign_key_exists?(referencing_table_name, target_table, name: :fk_referenced, column: :non_standard_id, on_delete: :nullify)).to be_falsey end end @@ -1110,7 +1114,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do end context 'specifying a target table' do - let(:target_table) { :referenced } + let(:target_table) { referenced_table_name } it_behaves_like 'foreign key checks' end @@ -1121,64 +1125,78 @@ RSpec.describe Gitlab::Database::MigrationHelpers do it 'raises an error if an invalid on_delete is specified' do # The correct on_delete key is "nullify" - expect { model.foreign_key_exists?(:referenced, on_delete: :set_null) }.to raise_error(ArgumentError) + expect { model.foreign_key_exists?(referenced_table_name, on_delete: :set_null) }.to raise_error(ArgumentError) end context 'with foreign key using multiple columns' do + let(:p_referenced_table_name) { '_test_gitlab_main_p_referenced' } + let(:p_referencing_table_name) { '_test_gitlab_main_p_referencing' } + before do model.connection.execute(<<~SQL) - create table p_referenced ( - id bigserial not null, - partition_number bigint not null default 100, - primary key (partition_number, id) - ); - create table p_referencing ( - id bigserial primary key not null, - partition_number bigint not null, - constraint fk_partitioning foreign key (partition_number, id) references p_referenced(partition_number, id) on delete cascade - ); + create table #{p_referenced_table_name} ( + id bigserial not null, + partition_number bigint not null default 100, + primary key (partition_number, id) + ); + create table #{p_referencing_table_name} ( + id bigserial primary key not null, + partition_number bigint not null, + constraint fk_partitioning foreign key (partition_number, id) + references #{p_referenced_table_name} (partition_number, id) on delete cascade + ); SQL end it 'finds existing foreign keys by columns' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, column: [:partition_number, :id])).to be_truthy + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + column: [:partition_number, :id])).to be_truthy end it 'finds existing foreign keys by name' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :fk_partitioning)).to be_truthy + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + name: :fk_partitioning)).to be_truthy end it 'finds existing foreign_keys by name and column' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :fk_partitioning, column: [:partition_number, :id])).to be_truthy + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + name: :fk_partitioning, column: [:partition_number, :id])).to be_truthy end it 'finds existing foreign_keys by name, column and on_delete' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :fk_partitioning, column: [:partition_number, :id], on_delete: :cascade)).to be_truthy + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + name: :fk_partitioning, column: [:partition_number, :id], on_delete: :cascade)).to be_truthy end it 'finds existing foreign keys by target table only' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced)).to be_truthy + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name)).to be_truthy end it 'compares by column name if given' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, column: :id)).to be_falsey + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + column: :id)).to be_falsey end it 'compares by target column name if given' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, primary_key: :user_id)).to be_falsey - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, primary_key: [:partition_number, :id])).to be_truthy + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + primary_key: :user_id)).to be_falsey + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + primary_key: [:partition_number, :id])).to be_truthy end it 'compares by foreign key name if given' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :non_existent_foreign_key_name)).to be_falsey + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + name: :non_existent_foreign_key_name)).to be_falsey end it 'compares by foreign key name and column if given' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :non_existent_foreign_key_name, column: [:partition_number, :id])).to be_falsey + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + name: :non_existent_foreign_key_name, column: [:partition_number, :id])).to be_falsey end it 'compares by foreign key name, column and on_delete if given' do - expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :fk_partitioning, column: [:partition_number, :id], on_delete: :nullify)).to be_falsey + expect(model.foreign_key_exists?(p_referencing_table_name, p_referenced_table_name, + name: :fk_partitioning, column: [:partition_number, :id], on_delete: :nullify)).to be_falsey end end end diff --git a/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb b/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb index 8027990a546..ac318ac2f49 100644 --- a/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb +++ b/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb @@ -6,22 +6,14 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do include Database::PartitioningHelpers include ExclusiveLeaseHelpers - def has_partition(model, month) - Gitlab::Database::PostgresPartition.for_parent_table(model.table_name).any? do |partition| - Gitlab::Database::Partitioning::TimePartition.from_sql( - model.table_name, - partition.name, - partition.condition - ).from == month - end - end + let(:partitioned_table_name) { "_test_gitlab_main_my_model_example_table" } context 'creating partitions (mocked)' do subject(:sync_partitions) { described_class.new(model).sync_partitions } let(:model) { double(partitioning_strategy: partitioning_strategy, table_name: table, connection: connection) } let(:connection) { ActiveRecord::Base.connection } - let(:table) { "my_model_example_table" } + let(:table) { partitioned_table_name } let(:partitioning_strategy) do double(missing_partitions: partitions, extra_partitions: [], after_adding_partitions: nil) end @@ -102,14 +94,14 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do Class.new(ApplicationRecord) do include PartitionedTable - self.table_name = 'my_model_example_table' - partitioned_by :created_at, strategy: :monthly end end before do - create_partitioned_table(connection, 'my_model_example_table') + my_model.table_name = partitioned_table_name + + create_partitioned_table(connection, partitioned_table_name) end it 'creates partitions' do @@ -184,27 +176,27 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do Class.new(ApplicationRecord) do include PartitionedTable - self.table_name = 'my_model_example_table' - partitioned_by :created_at, strategy: :monthly, retain_for: 1.month end end before do connection.execute(<<~SQL) - CREATE TABLE my_model_example_table + CREATE TABLE #{partitioned_table_name} (id serial not null, created_at timestamptz not null, primary key (id, created_at)) PARTITION BY RANGE (created_at); - CREATE TABLE #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.my_model_example_table_202104 - PARTITION OF my_model_example_table + CREATE TABLE #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.#{partitioned_table_name}_202104 + PARTITION OF #{partitioned_table_name} FOR VALUES FROM ('2021-04-01') TO ('2021-05-01'); - CREATE TABLE #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.my_model_example_table_202105 - PARTITION OF my_model_example_table + CREATE TABLE #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.#{partitioned_table_name}_202105 + PARTITION OF #{partitioned_table_name} FOR VALUES FROM ('2021-05-01') TO ('2021-06-01'); SQL + my_model.table_name = partitioned_table_name + # Also create all future partitions so that the sync is only trying to detach old partitions my_model.partitioning_strategy.missing_partitions.each do |p| connection.execute p.to_sql @@ -234,7 +226,7 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do it 'creates the appropriate PendingPartitionDrop entry' do subject - pending_drop = Postgresql::DetachedPartition.find_by!(table_name: 'my_model_example_table_202104') + pending_drop = Postgresql::DetachedPartition.find_by!(table_name: "#{partitioned_table_name}_202104") expect(pending_drop.drop_after).to eq(Time.current + described_class::RETAIN_DETACHED_PARTITIONS_FOR) end @@ -243,11 +235,11 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do context 'when the model is the target of a foreign key' do before do connection.execute(<<~SQL) - create unique index idx_for_fk ON my_model_example_table(created_at); + create unique index idx_for_fk ON #{partitioned_table_name}(created_at); - create table referencing_table ( + create table _test_gitlab_main_referencing_table ( id bigserial primary key not null, - referencing_created_at timestamptz references my_model_example_table(created_at) + referencing_created_at timestamptz references #{partitioned_table_name}(created_at) ); SQL end @@ -265,15 +257,15 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do Class.new(ApplicationRecord) do include PartitionedTable - self.table_name = 'my_model_example_table' - partitioned_by :created_at, strategy: :monthly, retain_for: 1.month end end before do + my_model.table_name = partitioned_table_name + connection.execute(<<~SQL) - CREATE TABLE my_model_example_table + CREATE TABLE #{partitioned_table_name} (id serial not null, created_at timestamptz not null, primary key (id, created_at)) PARTITION BY RANGE (created_at); SQL @@ -294,6 +286,16 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do end end + def has_partition(model, month) + Gitlab::Database::PostgresPartition.for_parent_table(model.table_name).any? do |partition| + Gitlab::Database::Partitioning::TimePartition.from_sql( + model.table_name, + partition.name, + partition.condition + ).from == month + end + end + def create_partitioned_table(connection, table) connection.execute(<<~SQL) CREATE TABLE #{table} diff --git a/spec/lib/gitlab/database/postgres_foreign_key_spec.rb b/spec/lib/gitlab/database/postgres_foreign_key_spec.rb index 5252d391305..ae56f66737d 100644 --- a/spec/lib/gitlab/database/postgres_foreign_key_spec.rb +++ b/spec/lib/gitlab/database/postgres_foreign_key_spec.rb @@ -6,26 +6,32 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ # PostgresForeignKey does not `behaves_like 'a postgres model'` because it does not correspond 1-1 with a single entry # in pg_class + let(:table_prefix) { '_test_gitlab_main' } + before do ApplicationRecord.connection.execute(<<~SQL) - CREATE TABLE public.referenced_table ( + CREATE TABLE #{schema_table_name('referenced_table')} ( id bigserial primary key not null, id_b bigserial not null, UNIQUE (id, id_b) ); - CREATE TABLE public.other_referenced_table ( + CREATE TABLE #{schema_table_name('other_referenced_table')} ( id bigserial primary key not null ); - CREATE TABLE public.constrained_table ( + CREATE TABLE #{schema_table_name('constrained_table')} ( id bigserial primary key not null, referenced_table_id bigint not null, referenced_table_id_b bigint not null, other_referenced_table_id bigint not null, - CONSTRAINT fk_constrained_to_referenced FOREIGN KEY(referenced_table_id, referenced_table_id_b) REFERENCES referenced_table(id, id_b) on delete restrict on update restrict, + CONSTRAINT fk_constrained_to_referenced + FOREIGN KEY(referenced_table_id, referenced_table_id_b) + REFERENCES #{table_name('referenced_table')}(id, id_b) + ON DELETE restrict + ON UPDATE restrict, CONSTRAINT fk_constrained_to_other_referenced FOREIGN KEY(other_referenced_table_id) - REFERENCES other_referenced_table(id) + REFERENCES #{table_name('other_referenced_table')}(id) ); SQL @@ -33,13 +39,13 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ describe '#by_referenced_table_identifier' do it 'throws an error when the identifier name is not fully qualified' do - expect { described_class.by_referenced_table_identifier('referenced_table') }.to raise_error(ArgumentError, /not fully qualified/) + expect { described_class.by_referenced_table_identifier(table_name("referenced_table")) }.to raise_error(ArgumentError, /not fully qualified/) end it 'finds the foreign keys for the referenced table' do expected = described_class.find_by!(name: 'fk_constrained_to_referenced') - expect(described_class.by_referenced_table_identifier('public.referenced_table')).to contain_exactly(expected) + expect(described_class.by_referenced_table_identifier(schema_table_name("referenced_table"))).to contain_exactly(expected) end end @@ -47,19 +53,19 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ it 'finds the foreign keys for the referenced table' do expected = described_class.find_by!(name: 'fk_constrained_to_referenced') - expect(described_class.by_referenced_table_name('referenced_table')).to contain_exactly(expected) + expect(described_class.by_referenced_table_name(table_name("referenced_table"))).to contain_exactly(expected) end end describe '#by_constrained_table_identifier' do it 'throws an error when the identifier name is not fully qualified' do - expect { described_class.by_constrained_table_identifier('constrained_table') }.to raise_error(ArgumentError, /not fully qualified/) + expect { described_class.by_constrained_table_identifier(table_name("constrained_table")) }.to raise_error(ArgumentError, /not fully qualified/) end it 'finds the foreign keys for the constrained table' do expected = described_class.where(name: %w[fk_constrained_to_referenced fk_constrained_to_other_referenced]).to_a - expect(described_class.by_constrained_table_identifier('public.constrained_table')).to match_array(expected) + expect(described_class.by_constrained_table_identifier(schema_table_name('constrained_table'))).to match_array(expected) end end @@ -67,7 +73,7 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ it 'finds the foreign keys for the constrained table' do expected = described_class.where(name: %w[fk_constrained_to_referenced fk_constrained_to_other_referenced]).to_a - expect(described_class.by_constrained_table_name('constrained_table')).to match_array(expected) + expect(described_class.by_constrained_table_name(table_name("constrained_table"))).to match_array(expected) end end @@ -80,7 +86,7 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ context 'when finding columns for foreign keys' do using RSpec::Parameterized::TableSyntax - let(:fks) { described_class.by_constrained_table_name('constrained_table') } + let(:fks) { described_class.by_constrained_table_name(table_name("constrained_table")) } where(:fk, :expected_constrained, :expected_referenced) do lazy { described_class.find_by(name: 'fk_constrained_to_referenced') } | %w[referenced_table_id referenced_table_id_b] | %w[id id_b] @@ -113,22 +119,31 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ describe '#on_delete_action and #on_update_action' do before do ApplicationRecord.connection.execute(<<~SQL) - create table public.referenced_table_all_on_delete_actions ( + create table #{schema_table_name('referenced_table_all_on_delete_actions')} ( id bigserial primary key not null ); - create table public.constrained_table_all_on_delete_actions ( + create table #{schema_table_name('constrained_table_all_on_delete_actions')} ( id bigserial primary key not null, - ref_id_no_action bigint not null constraint fk_no_action references referenced_table_all_on_delete_actions(id), - ref_id_restrict bigint not null constraint fk_restrict references referenced_table_all_on_delete_actions(id) on delete restrict on update restrict, - ref_id_nullify bigint not null constraint fk_nullify references referenced_table_all_on_delete_actions(id) on delete set null on update set null, - ref_id_cascade bigint not null constraint fk_cascade references referenced_table_all_on_delete_actions(id) on delete cascade on update cascade, - ref_id_set_default bigint not null constraint fk_set_default references referenced_table_all_on_delete_actions(id) on delete set default on update set default + ref_id_no_action bigint not null constraint fk_no_action + references #{table_name('referenced_table_all_on_delete_actions')}(id), + ref_id_restrict bigint not null constraint fk_restrict + references #{table_name('referenced_table_all_on_delete_actions')}(id) + on delete restrict on update restrict, + ref_id_nullify bigint not null constraint fk_nullify + references #{table_name('referenced_table_all_on_delete_actions')}(id) + on delete set null on update set null, + ref_id_cascade bigint not null constraint fk_cascade + references #{table_name('referenced_table_all_on_delete_actions')}(id) + on delete cascade on update cascade, + ref_id_set_default bigint not null constraint fk_set_default + references #{table_name('referenced_table_all_on_delete_actions')}(id) + on delete set default on update set default ) SQL end - let(:fks) { described_class.by_constrained_table_name('constrained_table_all_on_delete_actions') } + let(:fks) { described_class.by_constrained_table_name(table_name('constrained_table_all_on_delete_actions')) } context 'with an invalid on_delete_action' do it 'raises an error' do @@ -177,16 +192,17 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ skip('not supported before postgres 12') if ApplicationRecord.database.version.to_f < 12 ApplicationRecord.connection.execute(<<~SQL) - create table public.parent ( - id bigserial primary key not null - ) partition by hash(id); + create table #{schema_table_name('parent')} ( + id bigserial primary key not null + ) partition by hash(id); - create table public.child partition of parent for values with (modulus 2, remainder 1); + create table #{schema_table_name('child')} partition of #{table_name('parent')} + for values with (modulus 2, remainder 1); - create table public.referencing_partitioned ( - id bigserial not null primary key, - constraint fk_inherited foreign key (id) references parent(id) - ) + create table #{schema_table_name('referencing_partitioned')} ( + id bigserial not null primary key, + constraint fk_inherited foreign key (id) references #{table_name('parent')}(id) + ) SQL end @@ -195,7 +211,7 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ where(:fk, :inherited) do lazy { described_class.find_by(name: 'fk_inherited') } | false - lazy { described_class.by_referenced_table_identifier('public.child').first! } | true + lazy { described_class.by_referenced_table_identifier(schema_table_name('child')).first! } | true lazy { described_class.find_by(name: 'fk_constrained_to_referenced') } | false end @@ -207,12 +223,20 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ end describe '#not_inherited' do - let(:fks) { described_class.by_constrained_table_identifier('public.referencing_partitioned') } + let(:fks) { described_class.by_constrained_table_identifier(schema_table_name('referencing_partitioned')) } it 'lists all non-inherited foreign keys' do - expect(fks.pluck(:referenced_table_name)).to contain_exactly('parent', 'child') - expect(fks.not_inherited.pluck(:referenced_table_name)).to contain_exactly('parent') + expect(fks.pluck(:referenced_table_name)).to contain_exactly(table_name('parent'), table_name('child')) + expect(fks.not_inherited.pluck(:referenced_table_name)).to contain_exactly(table_name('parent')) end end end + + def schema_table_name(name) + "public.#{table_name(name)}" + end + + def table_name(name) + "#{table_prefix}_#{name}" + end end diff --git a/spec/lib/gitlab/database/postgres_index_spec.rb b/spec/lib/gitlab/database/postgres_index_spec.rb index db66736676b..d8a2612caf3 100644 --- a/spec/lib/gitlab/database/postgres_index_spec.rb +++ b/spec/lib/gitlab/database/postgres_index_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Gitlab::Database::PostgresIndex do CREATE INDEX #{name} ON public.users (name); CREATE UNIQUE INDEX bar_key ON public.users (id); - CREATE TABLE example_table (id serial primary key); + CREATE TABLE _test_gitlab_main_example_table (id serial primary key); SQL end @@ -144,7 +144,7 @@ RSpec.describe Gitlab::Database::PostgresIndex do end it 'returns true for a primary key index' do - expect(find('public.example_table_pkey')).to be_unique + expect(find('public._test_gitlab_main_example_table_pkey')).to be_unique end end diff --git a/spec/lib/gitlab/database/postgres_partitioned_table_spec.rb b/spec/lib/gitlab/database/postgres_partitioned_table_spec.rb index 21a46f1a0a6..23cf7329b73 100644 --- a/spec/lib/gitlab/database/postgres_partitioned_table_spec.rb +++ b/spec/lib/gitlab/database/postgres_partitioned_table_spec.rb @@ -3,25 +3,29 @@ require 'spec_helper' RSpec.describe Gitlab::Database::PostgresPartitionedTable, type: :model do - let(:schema) { 'public' } - let(:name) { 'foo_range' } - let(:identifier) { "#{schema}.#{name}" } + let_it_be(:foo_range_table_name) { '_test_gitlab_main_foo_range' } + let_it_be(:foo_list_table_name) { '_test_gitlab_main_foo_list' } + let_it_be(:foo_hash_table_name) { '_test_gitlab_main_foo_hash' } - before do + let_it_be(:schema) { 'public' } + let_it_be(:name) { foo_range_table_name } + let_it_be(:identifier) { "#{schema}.#{name}" } + + before_all do ActiveRecord::Base.connection.execute(<<~SQL) - CREATE TABLE #{identifier} ( + CREATE TABLE #{schema}.#{foo_range_table_name} ( id serial NOT NULL, created_at timestamptz NOT NULL, PRIMARY KEY (id, created_at) ) PARTITION BY RANGE(created_at); - CREATE TABLE public.foo_list ( + CREATE TABLE #{schema}.#{foo_list_table_name} ( id serial NOT NULL, row_type text NOT NULL, PRIMARY KEY (id, row_type) ) PARTITION BY LIST(row_type); - CREATE TABLE public.foo_hash ( + CREATE TABLE #{schema}.#{foo_hash_table_name} ( id serial NOT NULL, row_value int NOT NULL, PRIMARY KEY (id, row_value) @@ -58,29 +62,29 @@ RSpec.describe Gitlab::Database::PostgresPartitionedTable, type: :model do describe '#dynamic?' do it 'returns true for tables partitioned by range' do - expect(find('public.foo_range')).to be_dynamic + expect(find("#{schema}.#{foo_range_table_name}")).to be_dynamic end it 'returns true for tables partitioned by list' do - expect(find('public.foo_list')).to be_dynamic + expect(find("#{schema}.#{foo_list_table_name}")).to be_dynamic end it 'returns false for tables partitioned by hash' do - expect(find('public.foo_hash')).not_to be_dynamic + expect(find("#{schema}.#{foo_hash_table_name}")).not_to be_dynamic end end describe '#static?' do it 'returns false for tables partitioned by range' do - expect(find('public.foo_range')).not_to be_static + expect(find("#{schema}.#{foo_range_table_name}")).not_to be_static end it 'returns false for tables partitioned by list' do - expect(find('public.foo_list')).not_to be_static + expect(find("#{schema}.#{foo_list_table_name}")).not_to be_static end it 'returns true for tables partitioned by hash' do - expect(find('public.foo_hash')).to be_static + expect(find("#{schema}.#{foo_hash_table_name}")).to be_static end end diff --git a/spec/lib/gitlab/incident_management/pager_duty/incident_issue_description_spec.rb b/spec/lib/gitlab/incident_management/pager_duty/incident_issue_description_spec.rb index e2c67c68eb7..7573741082b 100644 --- a/spec/lib/gitlab/incident_management/pager_duty/incident_issue_description_spec.rb +++ b/spec/lib/gitlab/incident_management/pager_duty/incident_issue_description_spec.rb @@ -28,10 +28,10 @@ RSpec.describe Gitlab::IncidentManagement::PagerDuty::IncidentIssueDescription d } end - subject(:to_s) { described_class.new(incident_payload).to_s } + subject(:description) { described_class.new(incident_payload).to_s } it 'returns description' do - expect(to_s).to eq( + expect(description).to eq( <<~MARKDOWN.chomp **Incident:** [My new incident](https://webdemo.pagerduty.com/incidents/PRORDTY)#{markdown_line_break} **Incident number:** 33#{markdown_line_break} @@ -52,7 +52,7 @@ RSpec.describe Gitlab::IncidentManagement::PagerDuty::IncidentIssueDescription d freeze_time do now = Time.current.utc.strftime('%d %B %Y, %-l:%M%p (%Z)') - expect(to_s).to include( + expect(description).to include( <<~MARKDOWN.chomp **Created at:** #{now}#{markdown_line_break} MARKDOWN @@ -70,7 +70,7 @@ RSpec.describe Gitlab::IncidentManagement::PagerDuty::IncidentIssueDescription d end it 'assignees is a list of links' do - expect(to_s).to include( + expect(description).to include( <<~MARKDOWN.chomp **Assignees:** [Laura Haley](https://laura.pagerduty.com), [John Doe](https://john.pagerduty.com)#{markdown_line_break} MARKDOWN @@ -84,7 +84,7 @@ RSpec.describe Gitlab::IncidentManagement::PagerDuty::IncidentIssueDescription d end it 'impacted service is a single link' do - expect(to_s).to include( + expect(description).to include( <<~MARKDOWN.chomp **Impacted service:** [XDB Cluster](https://xdb.pagerduty.com) MARKDOWN diff --git a/spec/lib/gitlab/memory/watchdog/event_reporter_spec.rb b/spec/lib/gitlab/memory/watchdog/event_reporter_spec.rb index f667bc724d2..f1d241249e2 100644 --- a/spec/lib/gitlab/memory/watchdog/event_reporter_spec.rb +++ b/spec/lib/gitlab/memory/watchdog/event_reporter_spec.rb @@ -106,11 +106,12 @@ RSpec.describe Gitlab::Memory::Watchdog::EventReporter, feature_category: :appli it 'logs violation' do expect(logger).to receive(:warn) - .with( + .with({ pid: Process.pid, worker_id: 'worker_1', memwd_rss_bytes: 1024, - message: 'dummy_text') + message: 'dummy_text' + }) subject end diff --git a/spec/lib/gitlab/memory/watchdog_spec.rb b/spec/lib/gitlab/memory/watchdog_spec.rb index 0b2f24476d9..c47dcdf8445 100644 --- a/spec/lib/gitlab/memory/watchdog_spec.rb +++ b/spec/lib/gitlab/memory/watchdog_spec.rb @@ -60,10 +60,10 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, feature_category: it 'reports started event once' do expect(reporter).to receive(:started).once - .with( + .with({ memwd_handler_class: handler.class.name, memwd_sleep_time_s: sleep_time_seconds - ) + }) watchdog.call end @@ -77,11 +77,11 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, feature_category: context 'when no monitors are configured' do it 'reports stopped event once with correct reason' do expect(reporter).to receive(:stopped).once - .with( + .with({ memwd_handler_class: handler.class.name, memwd_sleep_time_s: sleep_time_seconds, memwd_reason: 'monitors are not configured' - ) + }) watchdog.call end @@ -96,11 +96,11 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, feature_category: it 'reports stopped event once' do expect(reporter).to receive(:stopped).once - .with( + .with({ memwd_handler_class: handler.class.name, memwd_sleep_time_s: sleep_time_seconds, memwd_reason: 'background task stopped' - ) + }) watchdog.call end @@ -149,13 +149,13 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, feature_category: it 'reports strikes exceeded event' do expect(reporter).to receive(:strikes_exceeded) .with( - name, - memwd_handler_class: handler.class.name, - memwd_sleep_time_s: sleep_time_seconds, - memwd_cur_strikes: 1, - memwd_max_strikes: max_strikes, - message: "dummy_text" - ) + name, { + memwd_handler_class: handler.class.name, + memwd_sleep_time_s: sleep_time_seconds, + memwd_cur_strikes: 1, + memwd_max_strikes: max_strikes, + message: "dummy_text" + }) watchdog.call end @@ -163,11 +163,11 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, feature_category: it 'executes handler and stops the watchdog' do expect(handler).to receive(:call).and_return(true) expect(reporter).to receive(:stopped).once - .with( + .with({ memwd_handler_class: handler.class.name, memwd_sleep_time_s: sleep_time_seconds, memwd_reason: 'successfully handled' - ) + }) watchdog.call end diff --git a/spec/lib/gitlab/omniauth_initializer_spec.rb b/spec/lib/gitlab/omniauth_initializer_spec.rb index cb1f3473fbd..a94191f310c 100644 --- a/spec/lib/gitlab/omniauth_initializer_spec.rb +++ b/spec/lib/gitlab/omniauth_initializer_spec.rb @@ -189,7 +189,7 @@ RSpec.describe Gitlab::OmniauthInitializer do it 'passes "args" hash as symbolized hash argument' do hash_config = { 'name' => 'hash', 'args' => { 'custom' => 'format' } } - expect(devise_config).to receive(:omniauth).with(:hash, custom: 'format') + expect(devise_config).to receive(:omniauth).with(:hash, { custom: 'format' }) subject.execute([hash_config]) end @@ -197,7 +197,7 @@ RSpec.describe Gitlab::OmniauthInitializer do it 'normalizes a String strategy_class' do hash_config = { 'name' => 'hash', 'args' => { strategy_class: 'OmniAuth::Strategies::OAuth2Generic' } } - expect(devise_config).to receive(:omniauth).with(:hash, strategy_class: OmniAuth::Strategies::OAuth2Generic) + expect(devise_config).to receive(:omniauth).with(:hash, { strategy_class: OmniAuth::Strategies::OAuth2Generic }) subject.execute([hash_config]) end @@ -205,7 +205,7 @@ RSpec.describe Gitlab::OmniauthInitializer do it 'allows a class to be specified in strategy_class' do hash_config = { 'name' => 'hash', 'args' => { strategy_class: OmniAuth::Strategies::OAuth2Generic } } - expect(devise_config).to receive(:omniauth).with(:hash, strategy_class: OmniAuth::Strategies::OAuth2Generic) + expect(devise_config).to receive(:omniauth).with(:hash, { strategy_class: OmniAuth::Strategies::OAuth2Generic }) subject.execute([hash_config]) end @@ -219,7 +219,7 @@ RSpec.describe Gitlab::OmniauthInitializer do it 'configures remote_sign_out_handler proc for authentiq' do authentiq_config = { 'name' => 'authentiq', 'args' => {} } - expect(devise_config).to receive(:omniauth).with(:authentiq, remote_sign_out_handler: an_instance_of(Proc)) + expect(devise_config).to receive(:omniauth).with(:authentiq, { remote_sign_out_handler: an_instance_of(Proc) }) subject.execute([authentiq_config]) end @@ -227,7 +227,7 @@ RSpec.describe Gitlab::OmniauthInitializer do it 'configures on_single_sign_out proc for cas3' do cas3_config = { 'name' => 'cas3', 'args' => {} } - expect(devise_config).to receive(:omniauth).with(:cas3, on_single_sign_out: an_instance_of(Proc)) + expect(devise_config).to receive(:omniauth).with(:cas3, { on_single_sign_out: an_instance_of(Proc) }) subject.execute([cas3_config]) end @@ -239,11 +239,11 @@ RSpec.describe Gitlab::OmniauthInitializer do } expect(devise_config).to receive(:omniauth).with( - :google_oauth2, - access_type: "offline", - approval_prompt: "", - client_options: { connection_opts: { request: { timeout: Gitlab::OmniauthInitializer::OAUTH2_TIMEOUT_SECONDS } } } - ) + :google_oauth2, { + access_type: "offline", + approval_prompt: "", + client_options: { connection_opts: { request: { timeout: Gitlab::OmniauthInitializer::OAUTH2_TIMEOUT_SECONDS } } } + }) subject.execute([google_config]) end @@ -255,10 +255,10 @@ RSpec.describe Gitlab::OmniauthInitializer do } expect(devise_config).to receive(:omniauth).with( - :gitlab, - client_options: { site: conf.dig('args', 'client_options', 'site') }, - authorize_params: { gl_auth_type: 'login' } - ) + :gitlab, { + client_options: { site: conf.dig('args', 'client_options', 'site') }, + authorize_params: { gl_auth_type: 'login' } + }) subject.execute([conf]) end @@ -267,9 +267,9 @@ RSpec.describe Gitlab::OmniauthInitializer do conf = { 'name' => 'gitlab' } expect(devise_config).to receive(:omniauth).with( - :gitlab, - authorize_params: { gl_auth_type: 'login' } - ) + :gitlab, { + authorize_params: { gl_auth_type: 'login' } + }) subject.execute([conf]) end @@ -280,7 +280,7 @@ RSpec.describe Gitlab::OmniauthInitializer do expect(devise_config).to receive(:omniauth).with( :gitlab, 'a', - authorize_params: { gl_auth_type: 'login' } + { authorize_params: { gl_auth_type: 'login' } } ) subject.execute([conf]) @@ -295,7 +295,7 @@ RSpec.describe Gitlab::OmniauthInitializer do expect(devise_config).to receive(:omniauth).with( :gitlab, - authorize_params: { gl_auth_type: 'login' } + { authorize_params: { gl_auth_type: 'login' } } ) subject.execute([conf]) diff --git a/spec/services/audit_event_service_spec.rb b/spec/services/audit_event_service_spec.rb index 1d079adc0be..4f8b90fcb4a 100644 --- a/spec/services/audit_event_service_spec.rb +++ b/spec/services/audit_event_service_spec.rb @@ -13,12 +13,12 @@ RSpec.describe AuditEventService, :with_license do describe '#security_event' do it 'creates an event and logs to a file' do expect(service).to receive(:file_logger).and_return(logger) - expect(logger).to receive(:info).with(author_id: user.id, - author_name: user.name, - entity_id: project.id, - entity_type: "Project", - action: :destroy, - created_at: anything) + expect(logger).to receive(:info).with({ author_id: user.id, + author_name: user.name, + entity_id: project.id, + entity_type: "Project", + action: :destroy, + created_at: anything }) expect { service.security_event }.to change(AuditEvent, :count).by(1) end @@ -33,15 +33,15 @@ RSpec.describe AuditEventService, :with_license do target_id: 1 }) expect(service).to receive(:file_logger).and_return(logger) - expect(logger).to receive(:info).with(author_id: user.id, - author_name: user.name, - entity_type: 'Project', - entity_id: project.id, - from: 'true', - to: 'false', - action: :create, - target_id: 1, - created_at: anything) + expect(logger).to receive(:info).with({ author_id: user.id, + author_name: user.name, + entity_type: 'Project', + entity_id: project.id, + from: 'true', + to: 'false', + action: :create, + target_id: 1, + created_at: anything }) expect { service.security_event }.to change(AuditEvent, :count).by(1) @@ -58,12 +58,12 @@ RSpec.describe AuditEventService, :with_license do it 'is overridden successfully' do freeze_time do expect(service).to receive(:file_logger).and_return(logger) - expect(logger).to receive(:info).with(author_id: user.id, - author_name: user.name, - entity_id: project.id, - entity_type: "Project", - action: :destroy, - created_at: 3.weeks.ago) + expect(logger).to receive(:info).with({ author_id: user.id, + author_name: user.name, + entity_id: project.id, + entity_type: "Project", + action: :destroy, + created_at: 3.weeks.ago }) expect { service.security_event }.to change(AuditEvent, :count).by(1) expect(AuditEvent.last.created_at).to eq(3.weeks.ago) @@ -129,12 +129,12 @@ RSpec.describe AuditEventService, :with_license do describe '#log_security_event_to_file' do it 'logs security event to file' do expect(service).to receive(:file_logger).and_return(logger) - expect(logger).to receive(:info).with(author_id: user.id, - author_name: user.name, - entity_type: 'Project', - entity_id: project.id, - action: :destroy, - created_at: anything) + expect(logger).to receive(:info).with({ author_id: user.id, + author_name: user.name, + entity_type: 'Project', + entity_id: project.id, + action: :destroy, + created_at: anything }) service.log_security_event_to_file end diff --git a/spec/support/database/prevent_cross_joins.rb b/spec/support/database/prevent_cross_joins.rb index 42c69a26788..8e08824c464 100644 --- a/spec/support/database/prevent_cross_joins.rb +++ b/spec/support/database/prevent_cross_joins.rb @@ -23,6 +23,7 @@ module Database ALLOW_THREAD_KEY = :allow_cross_joins_across_databases ALLOW_ANNOTATE_KEY = ALLOW_THREAD_KEY.to_s.freeze + IGNORED_SCHEMAS = %i[gitlab_shared gitlab_internal].freeze def self.validate_cross_joins!(sql) return if Thread.current[ALLOW_THREAD_KEY] || sql.include?(ALLOW_ANNOTATE_KEY) @@ -40,8 +41,9 @@ module Database end schemas = ::Gitlab::Database::GitlabSchema.table_schemas(tables) + schemas.subtract(IGNORED_SCHEMAS) - if schemas.include?(:gitlab_ci) && schemas.include?(:gitlab_main) + if schemas.many? Thread.current[:has_cross_join_exception] = true raise CrossJoinAcrossUnsupportedTablesError, "Unsupported cross-join across '#{tables.join(", ")}' querying '#{schemas.to_a.join(", ")}' discovered " \ diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb index 072c660bc2b..143809e8f2a 100644 --- a/spec/workers/process_commit_worker_spec.rb +++ b/spec/workers/process_commit_worker_spec.rb @@ -137,7 +137,7 @@ RSpec.describe ProcessCommitWorker do context 'when issue has no first_mentioned_in_commit_at set' do it 'updates issue metrics' do - expect(update_metrics_and_reload) + expect { update_metrics_and_reload.call } .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date) end end @@ -148,7 +148,7 @@ RSpec.describe ProcessCommitWorker do end it "doesn't update issue metrics" do - expect(update_metrics_and_reload).not_to change { issue.metrics.first_mentioned_in_commit_at } + expect { update_metrics_and_reload.call }.not_to change { issue.metrics.first_mentioned_in_commit_at } end end @@ -158,7 +158,7 @@ RSpec.describe ProcessCommitWorker do end it "doesn't update issue metrics" do - expect(update_metrics_and_reload) + expect { update_metrics_and_reload.call } .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date) end end |