diff options
116 files changed, 797 insertions, 539 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index e23e3fd2982..5fea1768541 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.112.0 +0.113.0 @@ -418,7 +418,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.105.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.106.0', require: 'gitaly' gem 'grpc', '~> 1.11.0' # Locked until https://github.com/google/protobuf/issues/4210 is closed diff --git a/Gemfile.lock b/Gemfile.lock index 7f9207d9dfe..6415f9e6132 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -282,7 +282,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.105.0) + gitaly-proto (0.106.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1037,7 +1037,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.105.0) + gitaly-proto (~> 0.106.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 766f2479ea5..50ca0d5a729 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -285,7 +285,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.105.0) + gitaly-proto (0.106.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1047,7 +1047,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.105.0) + gitaly-proto (~> 0.106.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index 8139aa69fc7..e565af800d0 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -162,8 +162,10 @@ export default class Clusters { if (type === 'password') { this.tokenField.setAttribute('type', 'text'); + this.showTokenButton.textContent = s__('ClusterIntegration|Hide'); } else { this.tokenField.setAttribute('type', 'password'); + this.showTokenButton.textContent = s__('ClusterIntegration|Show'); } } diff --git a/app/assets/javascripts/vue_shared/components/icon.vue b/app/assets/javascripts/vue_shared/components/icon.vue index c42c4a1fbe7..e7ff76c8218 100644 --- a/app/assets/javascripts/vue_shared/components/icon.vue +++ b/app/assets/javascripts/vue_shared/components/icon.vue @@ -1,24 +1,40 @@ <script> -/* This is a re-usable vue component for rendering a svg sprite - icon - Sample configuration: - - <icon - name="retry" - :size="32" - css-classes="top" - /> - -*/ // only allow classes in images.scss e.g. s12 const validSizes = [8, 12, 16, 18, 24, 32, 48, 72]; +let iconValidator = () => true; + +/* + During development/tests we want to validate that we are just using icons that are actually defined +*/ +if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line global-require + const data = require('@gitlab-org/gitlab-svgs/dist/icons.json'); + const { icons } = data; + iconValidator = value => { + if (icons.includes(value)) { + return true; + } + // eslint-disable-next-line no-console + console.warn(`Icon '${value}' is not a known icon of @gitlab/gitlab-svg`); + return false; + }; +} +/** This is a re-usable vue component for rendering a svg sprite icon + * @example + * <icon + * name="retry" + * :size="32" + * css-classes="top" + * /> + */ export default { props: { name: { type: String, required: true, + validator: iconValidator, }, size: { @@ -83,6 +99,6 @@ export default { :x="x" :y="y" > - <use v-bind="{ 'xlink:href':spriteHref }" /> + <use v-bind="{ 'xlink:href':spriteHref }"/> </svg> </template> diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f45fcd4d900..eeceb99c8d2 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -11,6 +11,7 @@ class ApplicationController < ActionController::Base include EnforcesTwoFactorAuthentication include WithPerformanceBar + before_action :limit_unauthenticated_session_times before_action :authenticate_sessionless_user! before_action :authenticate_user! before_action :enforce_terms!, if: :should_enforce_terms? @@ -85,6 +86,24 @@ class ApplicationController < ActionController::Base end end + # By default, all sessions are given the same expiration time configured in + # the session store (e.g. 1 week). However, unauthenticated users can + # generate a lot of sessions, primarily for CSRF verification. It makes + # sense to reduce the TTL for unauthenticated to something much lower than + # the default (e.g. 1 hour) to limit Redis memory. In addition, Rails + # creates a new session after login, so the short TTL doesn't even need to + # be extended. + def limit_unauthenticated_session_times + return if current_user + + # Rack sets this header, but not all tests may have it: https://github.com/rack/rack/blob/fdcd03a3c5a1c51d1f96fc97f9dfa1a9deac0c77/lib/rack/session/abstract/id.rb#L251-L259 + return unless request.env['rack.session.options'] + + # This works because Rack uses these options every time a request is handled: + # https://github.com/rack/rack/blob/fdcd03a3c5a1c51d1f96fc97f9dfa1a9deac0c77/lib/rack/session/abstract/id.rb#L342 + request.env['rack.session.options'][:expire_after] = Settings.gitlab['unauthenticated_session_expire_delay'] + end + protected def append_info_to_payload(payload) diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb index 2f304b040c7..41084ec686f 100644 --- a/app/helpers/icons_helper.rb +++ b/app/helpers/icons_helper.rb @@ -1,3 +1,5 @@ +require 'json' + module IconsHelper extend self include FontAwesome::Rails::IconHelper @@ -38,6 +40,13 @@ module IconsHelper end def sprite_icon(icon_name, size: nil, css_class: nil) + if Gitlab::Sentry.should_raise? + unless known_sprites.include?(icon_name) + exception = ArgumentError.new("#{icon_name} is not a known icon in @gitlab-org/gitlab-svg") + raise exception + end + end + css_classes = size ? "s#{size}" : "" css_classes << " #{css_class}" unless css_class.blank? content_tag(:svg, content_tag(:use, "", { "xlink:href" => "#{sprite_icon_path}##{icon_name}" } ), class: css_classes.empty? ? nil : css_classes) @@ -134,4 +143,10 @@ module IconsHelper icon_class end + + private + + def known_sprites + @known_sprites ||= JSON.parse(File.read(Rails.root.join('node_modules/@gitlab-org/gitlab-svgs/dist/icons.json')))['icons'] + end end diff --git a/app/models/user.rb b/app/models/user.rb index 4987d01aac6..58429f8d607 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -14,7 +14,6 @@ class User < ActiveRecord::Base include IgnorableColumn include FeatureGate include CreatedAtFilterable - include IgnorableColumn include BulkMemberAccessLoad include BlocksJsonSerialization include WithUploads diff --git a/app/services/prometheus/adapter_service.rb b/app/services/prometheus/adapter_service.rb index 4504d2ccfe6..cbba79690c5 100644 --- a/app/services/prometheus/adapter_service.rb +++ b/app/services/prometheus/adapter_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Prometheus class AdapterService def initialize(project, deployment_platform = nil) diff --git a/app/services/protected_branches/access_level_params.rb b/app/services/protected_branches/access_level_params.rb index 4658b0e850d..a7ef573ff0b 100644 --- a/app/services/protected_branches/access_level_params.rb +++ b/app/services/protected_branches/access_level_params.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ProtectedBranches class AccessLevelParams attr_reader :type, :params diff --git a/app/services/protected_branches/api_service.rb b/app/services/protected_branches/api_service.rb index 4b40200644b..4340d3e8260 100644 --- a/app/services/protected_branches/api_service.rb +++ b/app/services/protected_branches/api_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ProtectedBranches class ApiService < BaseService def create diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb index 9d947f73af1..87aaf4672a4 100644 --- a/app/services/protected_branches/create_service.rb +++ b/app/services/protected_branches/create_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ProtectedBranches class CreateService < BaseService def execute(skip_authorization: false) diff --git a/app/services/protected_branches/destroy_service.rb b/app/services/protected_branches/destroy_service.rb index 8172c896e76..7190bc2001b 100644 --- a/app/services/protected_branches/destroy_service.rb +++ b/app/services/protected_branches/destroy_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ProtectedBranches class DestroyService < BaseService def execute(protected_branch) diff --git a/app/services/protected_branches/legacy_api_create_service.rb b/app/services/protected_branches/legacy_api_create_service.rb index bb7656489c5..aef99a860a0 100644 --- a/app/services/protected_branches/legacy_api_create_service.rb +++ b/app/services/protected_branches/legacy_api_create_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # The branches#protect API still uses the `developers_can_push` and `developers_can_merge` # flags for backward compatibility, and so performs translation between that format and the # internal data model (separate access levels). The translation code is non-trivial, and so diff --git a/app/services/protected_branches/legacy_api_update_service.rb b/app/services/protected_branches/legacy_api_update_service.rb index 1df38de0e4a..1f6bbe72f85 100644 --- a/app/services/protected_branches/legacy_api_update_service.rb +++ b/app/services/protected_branches/legacy_api_update_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # The branches#protect API still uses the `developers_can_push` and `developers_can_merge` # flags for backward compatibility, and so performs translation between that format and the # internal data model (separate access levels). The translation code is non-trivial, and so diff --git a/app/services/protected_branches/update_service.rb b/app/services/protected_branches/update_service.rb index 95e46645374..4d7d498b8ca 100644 --- a/app/services/protected_branches/update_service.rb +++ b/app/services/protected_branches/update_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ProtectedBranches class UpdateService < BaseService def execute(protected_branch) diff --git a/app/services/protected_tags/create_service.rb b/app/services/protected_tags/create_service.rb index faba7865a17..9aff55986b2 100644 --- a/app/services/protected_tags/create_service.rb +++ b/app/services/protected_tags/create_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ProtectedTags class CreateService < BaseService attr_reader :protected_tag diff --git a/app/services/protected_tags/destroy_service.rb b/app/services/protected_tags/destroy_service.rb index c868d7ad8e6..dc4a1b39848 100644 --- a/app/services/protected_tags/destroy_service.rb +++ b/app/services/protected_tags/destroy_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ProtectedTags class DestroyService < BaseService def execute(protected_tag) diff --git a/app/services/protected_tags/update_service.rb b/app/services/protected_tags/update_service.rb index aea6a48968d..3eb5f4955ee 100644 --- a/app/services/protected_tags/update_service.rb +++ b/app/services/protected_tags/update_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ProtectedTags class UpdateService < BaseService def execute(protected_tag) diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 9ac8fdb4cff..cdc8514c47c 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module QuickActions class InterpretService < BaseService include Gitlab::QuickActions::Dsl diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb index 92a32e703af..cb1bf0a03a5 100644 --- a/app/services/search/global_service.rb +++ b/app/services/search/global_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Search class GlobalService attr_accessor :current_user, :params diff --git a/app/services/search/group_service.rb b/app/services/search/group_service.rb index b4efba68715..34803d005e3 100644 --- a/app/services/search/group_service.rb +++ b/app/services/search/group_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Search class GroupService < Search::GlobalService attr_accessor :group diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb index 9a22abae635..f223c8be103 100644 --- a/app/services/search/project_service.rb +++ b/app/services/search/project_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Search class ProjectService attr_accessor :project, :current_user, :params diff --git a/app/services/search/snippet_service.rb b/app/services/search/snippet_service.rb index 85da0be6fff..e899a36f468 100644 --- a/app/services/search/snippet_service.rb +++ b/app/services/search/snippet_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Search class SnippetService attr_accessor :current_user, :params diff --git a/app/services/tags/create_service.rb b/app/services/tags/create_service.rb index 3cc88d77ba1..329722df747 100644 --- a/app/services/tags/create_service.rb +++ b/app/services/tags/create_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Tags class CreateService < BaseService def execute(tag_name, target, message, release_description = nil) diff --git a/app/services/tags/destroy_service.rb b/app/services/tags/destroy_service.rb index b84b061e460..800268485a4 100644 --- a/app/services/tags/destroy_service.rb +++ b/app/services/tags/destroy_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Tags class DestroyService < BaseService def execute(tag_name) diff --git a/app/services/test_hooks/base_service.rb b/app/services/test_hooks/base_service.rb index aadc1ea644b..8b5439c00bf 100644 --- a/app/services/test_hooks/base_service.rb +++ b/app/services/test_hooks/base_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module TestHooks class BaseService attr_accessor :hook, :current_user, :trigger diff --git a/app/services/test_hooks/project_service.rb b/app/services/test_hooks/project_service.rb index 65183e84cce..45e0e61e5c4 100644 --- a/app/services/test_hooks/project_service.rb +++ b/app/services/test_hooks/project_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module TestHooks class ProjectService < TestHooks::BaseService attr_writer :project diff --git a/app/services/test_hooks/system_service.rb b/app/services/test_hooks/system_service.rb index 9016c77b7f0..082830c5538 100644 --- a/app/services/test_hooks/system_service.rb +++ b/app/services/test_hooks/system_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module TestHooks class SystemService < TestHooks::BaseService private diff --git a/app/services/users/activity_service.rb b/app/services/users/activity_service.rb index 5803404c3c8..822df6c646a 100644 --- a/app/services/users/activity_service.rb +++ b/app/services/users/activity_service.rb @@ -1,12 +1,21 @@ +# frozen_string_literal: true + module Users class ActivityService + LEASE_TIMEOUT = 1.minute.to_i + def initialize(author, activity) - @author = author.respond_to?(:user) ? author.user : author + @user = if author.respond_to?(:username) + author + elsif author.respond_to?(:user) + author.user + end + @activity = activity end def execute - return unless @author && @author.is_a?(User) + return unless @user record_activity end @@ -14,9 +23,14 @@ module Users private def record_activity - Gitlab::UserActivities.record(@author.id) if Gitlab::Database.read_write? + return if Gitlab::Database.read_only? + + lease = Gitlab::ExclusiveLease.new("acitvity_service:#{@user.id}", + timeout: LEASE_TIMEOUT) + return unless lease.try_obtain - Rails.logger.debug("Recorded activity: #{@activity} for User ID: #{@author.id} (username: #{@author.username})") + @user.update_attribute(:last_activity_on, Date.today) + Rails.logger.debug("Recorded activity: #{@activity} for User ID: #{@user.id} (username: #{@user.username})") end end end diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 4fb6d221909..c69b46cab5a 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Users class BuildService < BaseService def initialize(current_user, params = {}) diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb index c8a3c461d60..2ac6dfd90fa 100644 --- a/app/services/users/create_service.rb +++ b/app/services/users/create_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Users class CreateService < BaseService include NewUserNotifier diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 06b604dad4d..4bc78b5b64e 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Users class DestroyService attr_accessor :current_user diff --git a/app/services/users/last_push_event_service.rb b/app/services/users/last_push_event_service.rb index 57e446d7f30..a9c9497520b 100644 --- a/app/services/users/last_push_event_service.rb +++ b/app/services/users/last_push_event_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Users # Service class for caching and retrieving the last push event of a user. class LastPushEventService diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb index a2833b1e051..4d47078bf43 100644 --- a/app/services/users/migrate_to_ghost_user_service.rb +++ b/app/services/users/migrate_to_ghost_user_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # When a user is destroyed, some of their associated records are # moved to a "Ghost User", to prevent these associated records from # being destroyed. diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index f028f5eb0d4..23b63aaabdf 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Users # Service for refreshing the authorized projects of a user. # diff --git a/app/services/users/respond_to_terms_service.rb b/app/services/users/respond_to_terms_service.rb index 06d660186cf..9efa3b285a8 100644 --- a/app/services/users/respond_to_terms_service.rb +++ b/app/services/users/respond_to_terms_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Users class RespondToTermsService def initialize(user, term) diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index 15ca1a55a5b..6dadb5a4eac 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Users class UpdateService < BaseService include NewUserNotifier diff --git a/app/services/wiki_pages/base_service.rb b/app/services/wiki_pages/base_service.rb index 260c04a8b94..e259f5bd1bc 100644 --- a/app/services/wiki_pages/base_service.rb +++ b/app/services/wiki_pages/base_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module WikiPages class BaseService < ::BaseService private diff --git a/app/services/wiki_pages/create_service.rb b/app/services/wiki_pages/create_service.rb index 24a817c06c9..2e2e0fd9033 100644 --- a/app/services/wiki_pages/create_service.rb +++ b/app/services/wiki_pages/create_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module WikiPages class CreateService < WikiPages::BaseService def execute diff --git a/app/services/wiki_pages/destroy_service.rb b/app/services/wiki_pages/destroy_service.rb index 6b93fb2f6d7..3f9343339cd 100644 --- a/app/services/wiki_pages/destroy_service.rb +++ b/app/services/wiki_pages/destroy_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module WikiPages class DestroyService < WikiPages::BaseService def execute(page) diff --git a/app/services/wiki_pages/update_service.rb b/app/services/wiki_pages/update_service.rb index 93cbd9a509f..2159dd91e9c 100644 --- a/app/services/wiki_pages/update_service.rb +++ b/app/services/wiki_pages/update_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module WikiPages class UpdateService < WikiPages::BaseService def execute(page) diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml index e35ebdea435..6950e2e332d 100644 --- a/app/views/profiles/two_factor_auths/show.html.haml +++ b/app/views/profiles/two_factor_auths/show.html.haml @@ -13,10 +13,16 @@ - if current_user.two_factor_otp_enabled? %p You've already enabled two-factor authentication using mobile authenticator applications. In order to register a different device, you must first disable two-factor authentication. + %p + If you lose your recovery codes you can generate new ones, invalidating all previous codes. + %div = link_to 'Disable two-factor authentication', profile_two_factor_auth_path, method: :delete, data: { confirm: "Are you sure? This will invalidate your registered applications and U2F devices." }, - class: 'btn btn-danger' + class: 'btn btn-danger append-right-10' + = form_tag codes_profile_two_factor_auth_path, {style: 'display: inline-block', method: :post} do |f| + = submit_tag 'Regenerate recovery codes', class: 'btn' + - else %p Download the Google Authenticator application from App Store or Google Play Store and scan this code. diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index d4be1ccfcfa..4de35b9bd06 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -13,7 +13,6 @@ - cronjob:repository_archive_cache - cronjob:repository_check_dispatch - cronjob:requests_profiles -- cronjob:schedule_update_user_activity - cronjob:stuck_ci_jobs - cronjob:stuck_import_jobs - cronjob:stuck_merge_jobs @@ -114,7 +113,6 @@ - storage_migrator - system_hook_push - update_merge_requests -- update_user_activity - upload_checksum - web_hook - repository_update_remote_mirror diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb index 8d0cfc73ccd..17ad1d5ab88 100644 --- a/app/workers/emails_on_push_worker.rb +++ b/app/workers/emails_on_push_worker.rb @@ -51,7 +51,7 @@ class EmailsOnPushWorker end end - recipients.split.each do |recipient| + valid_recipients(recipients).each do |recipient| begin send_email( recipient, @@ -89,4 +89,10 @@ class EmailsOnPushWorker email.header[:skip_premailer] = true if skip_premailer email.deliver_now end + + def valid_recipients(recipients) + recipients.split.select do |recipient| + recipient.include?('@') + end + end end diff --git a/app/workers/schedule_update_user_activity_worker.rb b/app/workers/schedule_update_user_activity_worker.rb deleted file mode 100644 index ff42fb8f0e5..00000000000 --- a/app/workers/schedule_update_user_activity_worker.rb +++ /dev/null @@ -1,12 +0,0 @@ -# frozen_string_literal: true - -class ScheduleUpdateUserActivityWorker - include ApplicationWorker - include CronjobQueue - - def perform(batch_size = 500) - Gitlab::UserActivities.new.each_slice(batch_size) do |batch| - UpdateUserActivityWorker.perform_async(Hash[batch]) - end - end -end diff --git a/app/workers/update_user_activity_worker.rb b/app/workers/update_user_activity_worker.rb deleted file mode 100644 index 15f01a70337..00000000000 --- a/app/workers/update_user_activity_worker.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -class UpdateUserActivityWorker - include ApplicationWorker - - def perform(pairs) - pairs = cast_data(pairs) - ids = pairs.keys - conditions = 'WHEN id = ? THEN ? ' * ids.length - - User.where(id: ids) - .update_all([ - "last_activity_on = CASE #{conditions} ELSE last_activity_on END", - *pairs.to_a.flatten - ]) - - Gitlab::UserActivities.new.delete(*ids) - end - - private - - def cast_data(pairs) - pairs.each_with_object({}) do |(key, value), new_pairs| - new_pairs[key.to_i] = Time.at(value.to_i).to_s(:db) - end - end -end diff --git a/changelogs/unreleased/43312-remove_user_activity_workers.yml b/changelogs/unreleased/43312-remove_user_activity_workers.yml new file mode 100644 index 00000000000..6dfd018e350 --- /dev/null +++ b/changelogs/unreleased/43312-remove_user_activity_workers.yml @@ -0,0 +1,5 @@ +--- +title: Delete UserActivities and related workers +merge_request: 20597 +author: +type: performance diff --git a/changelogs/unreleased/accept-rf3-2822-compliant-addresses.yml b/changelogs/unreleased/accept-rf3-2822-compliant-addresses.yml new file mode 100644 index 00000000000..97d017613ba --- /dev/null +++ b/changelogs/unreleased/accept-rf3-2822-compliant-addresses.yml @@ -0,0 +1,5 @@ +--- +title: Emails on push recipients now accepts formats like John Doe <johndoe@example.com> +merge_request: +author: George Thomas +type: added diff --git a/changelogs/unreleased/add-total-time-flat-printer-for-profiling.yml b/changelogs/unreleased/add-total-time-flat-printer-for-profiling.yml new file mode 100644 index 00000000000..37a4e31896e --- /dev/null +++ b/changelogs/unreleased/add-total-time-flat-printer-for-profiling.yml @@ -0,0 +1,6 @@ +--- +title: Add a Gitlab::Profiler.print_by_total_time convenience method for profiling + from a Rails console +merge_request: +author: +type: other diff --git a/changelogs/unreleased/frozen-string-enable-apps-services-inner-even-more.yml b/changelogs/unreleased/frozen-string-enable-apps-services-inner-even-more.yml new file mode 100644 index 00000000000..cee790a07ff --- /dev/null +++ b/changelogs/unreleased/frozen-string-enable-apps-services-inner-even-more.yml @@ -0,0 +1,5 @@ +--- +title: Enable even more frozen string in app/services/**/*.rb +merge_request: 20702 +author: gfyoung +type: performance diff --git a/changelogs/unreleased/gitaly-ff-branch-nil.yml b/changelogs/unreleased/gitaly-ff-branch-nil.yml new file mode 100644 index 00000000000..e7e689e6053 --- /dev/null +++ b/changelogs/unreleased/gitaly-ff-branch-nil.yml @@ -0,0 +1,5 @@ +--- +title: Add missing Gitaly branch_update nil checks +merge_request: 20711 +author: +type: fixed diff --git a/changelogs/unreleased/rails5-fix-revert-modal-spec.yml b/changelogs/unreleased/rails5-fix-revert-modal-spec.yml new file mode 100644 index 00000000000..0637e503ca9 --- /dev/null +++ b/changelogs/unreleased/rails5-fix-revert-modal-spec.yml @@ -0,0 +1,5 @@ +--- +title: Rails5 fix user sees revert modal spec +merge_request: 20706 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/regen-2fa-codes.yml b/changelogs/unreleased/regen-2fa-codes.yml new file mode 100644 index 00000000000..596f759df0f --- /dev/null +++ b/changelogs/unreleased/regen-2fa-codes.yml @@ -0,0 +1,5 @@ +--- +title: Added button to regenerate 2FA codes +merge_request: +author: Luke Picciau +type: added diff --git a/changelogs/unreleased/sh-limit-unauthenticated-session-times.yml b/changelogs/unreleased/sh-limit-unauthenticated-session-times.yml new file mode 100644 index 00000000000..44a46b4115e --- /dev/null +++ b/changelogs/unreleased/sh-limit-unauthenticated-session-times.yml @@ -0,0 +1,5 @@ +--- +title: Limit the TTL for anonymous sessions to 1 hour +merge_request: 20700 +author: +type: performance diff --git a/changelogs/unreleased/sh-normalize-urls.yml b/changelogs/unreleased/sh-normalize-urls.yml new file mode 100644 index 00000000000..b0d1120e10b --- /dev/null +++ b/changelogs/unreleased/sh-normalize-urls.yml @@ -0,0 +1,5 @@ +--- +title: Escape username and password in UrlSanitizer#full_url +merge_request: 20684 +author: +type: fixed diff --git a/changelogs/unreleased/toggle-password-cluster.yml b/changelogs/unreleased/toggle-password-cluster.yml new file mode 100644 index 00000000000..1a43c4baa25 --- /dev/null +++ b/changelogs/unreleased/toggle-password-cluster.yml @@ -0,0 +1,5 @@ +--- +title: Toggle Show / Hide Button for Kubernetes Password +merge_request: 20659 +author: gfyoung +type: fixed diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 4b9cc59ec45..c3122827a6b 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -140,6 +140,7 @@ Settings.gitlab['default_projects_features'] ||= {} Settings.gitlab['webhook_timeout'] ||= 10 Settings.gitlab['max_attachment_size'] ||= 10 Settings.gitlab['session_expire_delay'] ||= 10080 +Settings.gitlab['unauthenticated_session_expire_delay'] ||= 1.hour.to_i Settings.gitlab.default_projects_features['issues'] = true if Settings.gitlab.default_projects_features['issues'].nil? Settings.gitlab.default_projects_features['merge_requests'] = true if Settings.gitlab.default_projects_features['merge_requests'].nil? Settings.gitlab.default_projects_features['wiki'] = true if Settings.gitlab.default_projects_features['wiki'].nil? @@ -318,10 +319,6 @@ Settings.cron_jobs['gitlab_usage_ping_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['gitlab_usage_ping_worker']['cron'] ||= Settings.__send__(:cron_for_usage_ping) Settings.cron_jobs['gitlab_usage_ping_worker']['job_class'] = 'GitlabUsagePingWorker' -Settings.cron_jobs['schedule_update_user_activity_worker'] ||= Settingslogic.new({}) -Settings.cron_jobs['schedule_update_user_activity_worker']['cron'] ||= '30 0 * * *' -Settings.cron_jobs['schedule_update_user_activity_worker']['job_class'] = 'ScheduleUpdateUserActivityWorker' - Settings.cron_jobs['remove_old_web_hook_logs_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['remove_old_web_hook_logs_worker']['cron'] ||= '40 0 * * *' Settings.cron_jobs['remove_old_web_hook_logs_worker']['job_class'] = 'RemoveOldWebHookLogsWorker' diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 3400142db36..70b584ff9e9 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -62,7 +62,6 @@ - [default, 1] - [pages, 1] - [system_hook_push, 1] - - [update_user_activity, 1] - [propagate_service_template, 1] - [background_migration, 1] - [gcp_cluster, 1] @@ -77,4 +76,3 @@ - [repository_remove_remote, 1] - [create_note_diff_file, 1] - [delete_diff_files, 1] - diff --git a/doc/api/projects.md b/doc/api/projects.md index a35c2a56992..f3ccca46420 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -55,6 +55,8 @@ GET /projects | `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | | `with_issues_enabled` | boolean | no | Limit by enabled issues feature | | `with_merge_requests_enabled` | boolean | no | Limit by enabled merge requests feature | +| `wiki_checksum_failed` | boolean | no | Limit projects where the wiki checksum calculation has failed _([Introduced][ee-6137] in [GitLab Premium][eep] 11.2)_ | +| `repository_checksum_failed` | boolean | no | Limit projects where the repository checksum calculation has failed _([Introduced][ee-6137] in [GitLab Premium][eep] 11.2)_ | When `simple=true` or the user is unauthenticated this returns something like: @@ -1509,3 +1511,6 @@ GET /projects/:id/snapshot | --------- | ---- | -------- | ----------- | | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | | `wiki` | boolean | no | Whether to download the wiki, rather than project, repository | + +[eep]: https://about.gitlab.com/pricing/ "Available only in GitLab Premium" +[ee-6137]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/6137 diff --git a/doc/development/profiling.md b/doc/development/profiling.md index 11878b4009b..0ca8bb67a77 100644 --- a/doc/development/profiling.md +++ b/doc/development/profiling.md @@ -42,6 +42,36 @@ Passing a `logger:` keyword argument to `Gitlab::Profiler.profile` will send ActiveRecord and ActionController log output to that logger. Further options are documented with the method source. +There is also a RubyProf printer available: +`Gitlab::Profiler::TotalTimeFlatPrinter`. This acts like +`RubyProf::FlatPrinter`, but its `min_percent` option works on the method's +total time, not its self time. (This is because we often spend most of our time +in library code, but this comes from calls in our application.) It also offers a +`max_percent` option to help filter out outer calls that aren't useful (like +`ActionDispatch::Integration::Session#process`). + +There is a convenience method for using this, +`Gitlab::Profiler.print_by_total_time`: + +```ruby +result = Gitlab::Profiler.profile('/my-user') +Gitlab::Profiler.print_by_total_time(result, max_percent: 60, min_percent: 2) +# Measure Mode: wall_time +# Thread ID: 70005223698240 +# Fiber ID: 70004894952580 +# Total: 1.768912 +# Sort by: total_time +# +# %self total self wait child calls name +# 0.00 1.017 0.000 0.000 1.017 14 *ActionView::Helpers::RenderingHelper#render +# 0.00 1.017 0.000 0.000 1.017 14 *ActionView::Renderer#render_partial +# 0.00 1.017 0.000 0.000 1.017 14 *ActionView::PartialRenderer#render +# 0.00 1.007 0.000 0.000 1.007 14 *ActionView::PartialRenderer#render_partial +# 0.00 0.930 0.000 0.000 0.930 14 Hamlit::TemplateHandler#call +# 0.00 0.928 0.000 0.000 0.928 14 Temple::Engine#call +# 0.02 0.865 0.000 0.000 0.864 638 *Enumerable#inject +``` + [GitLab-Profiler](https://gitlab.com/gitlab-com/gitlab-profiler) is a project that builds on this to add some additional niceties, such as allowing configuration with a single Yaml file for multiple URLs, and uploading of the diff --git a/doc/user/project/integrations/emails_on_push.md b/doc/user/project/integrations/emails_on_push.md index c01da883562..b050c83fb72 100644 --- a/doc/user/project/integrations/emails_on_push.md +++ b/doc/user/project/integrations/emails_on_push.md @@ -6,7 +6,7 @@ every change that is pushed to your project. Navigate to the [Integrations page](project_services.md#accessing-the-project-services) and select the **Emails on push** service to configure it. -In the _Recipients_ area, provide a list of emails separated by commas. +In the _Recipients_ area, provide a list of emails separated by spaces or newlines. You can configure any of the following settings depending on your preference. diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 0888e3befac..889e3d4f819 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -9,6 +9,21 @@ module API before { authenticate_non_get! } helpers do + params :optional_filter_params_ee do + # EE::API::Projects would override this helper + end + + # EE::API::Projects would override this method + def apply_filters(projects) + projects = projects.with_issues_available_for_user(current_user) if params[:with_issues_enabled] + projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] + projects = projects.with_statistics if params[:statistics] + + projects + end + end + + helpers do params :statistics_params do optional :statistics, type: Boolean, default: false, desc: 'Include project statistics' end @@ -39,6 +54,8 @@ module API optional :membership, type: Boolean, default: false, desc: 'Limit by projects that the current user is a member of' optional :with_issues_enabled, type: Boolean, default: false, desc: 'Limit by enabled issues feature' optional :with_merge_requests_enabled, type: Boolean, default: false, desc: 'Limit by enabled merge requests feature' + + use :optional_filter_params_ee end params :create_params do @@ -52,9 +69,7 @@ module API def present_projects(projects, options = {}) projects = reorder_projects(projects) - projects = projects.with_issues_available_for_user(current_user) if params[:with_issues_enabled] - projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] - projects = projects.with_statistics if params[:statistics] + projects = apply_filters(projects) projects = paginate(projects) projects, options = with_custom_attributes(projects, options) diff --git a/lib/gitlab/git/operation_service.rb b/lib/gitlab/git/operation_service.rb index 280def182d5..57d748343be 100644 --- a/lib/gitlab/git/operation_service.rb +++ b/lib/gitlab/git/operation_service.rb @@ -8,6 +8,8 @@ module Gitlab alias_method :branch_created?, :branch_created def self.from_gitaly(branch_update) + return if branch_update.nil? + new( branch_update.commit_id, branch_update.repo_created, diff --git a/lib/gitlab/git/repository_mirroring.rb b/lib/gitlab/git/repository_mirroring.rb index 9faa62be28e..ef86d4a55ca 100644 --- a/lib/gitlab/git/repository_mirroring.rb +++ b/lib/gitlab/git/repository_mirroring.rb @@ -17,33 +17,19 @@ module Gitlab rugged.config["remote.#{remote_name}.prune"] = true end - def remote_tags(remote) - # Each line has this format: "dc872e9fa6963f8f03da6c8f6f264d0845d6b092\trefs/tags/v1.10.0\n" - # We want to convert it to: [{ 'v1.10.0' => 'dc872e9fa6963f8f03da6c8f6f264d0845d6b092' }, ...] - list_remote_tags(remote).map do |line| - target, path = line.strip.split("\t") - - # When the remote repo does not have tags. - if target.nil? || path.nil? - Rails.logger.info "Empty or invalid list of tags for remote: #{remote}. Output: #{output}" - break [] + def remote_branches(remote_name) + gitaly_migrate(:ref_find_all_remote_branches) do |is_enabled| + if is_enabled + gitaly_ref_client.remote_branches(remote_name) + else + rugged_remote_branches(remote_name) end - - name = path.split('/', 3).last - # We're only interested in tag references - # See: http://stackoverflow.com/questions/15472107/when-listing-git-ls-remote-why-theres-after-the-tag-name - next if name =~ /\^\{\}\Z/ - - target_commit = Gitlab::Git::Commit.find(self, target) - Gitlab::Git::Tag.new(self, { - name: name, - target: target, - target_commit: target_commit - }) - end.compact + end end - def remote_branches(remote_name) + private + + def rugged_remote_branches(remote_name) branches = [] rugged.references.each("refs/remotes/#{remote_name}/*").map do |ref| @@ -60,8 +46,6 @@ module Gitlab branches end - private - def set_remote_refmap(remote_name, refmap) Array(refmap).each_with_index do |refspec, i| refspec = REFMAPS[refspec] || refspec @@ -75,21 +59,6 @@ module Gitlab end end end - - def list_remote_tags(remote) - tag_list, exit_code, error = nil - cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{path} ls-remote --tags #{remote}) - - Open3.popen3(*cmd) do |stdin, stdout, stderr, wait_thr| - tag_list = stdout.read - error = stderr.read - exit_code = wait_thr.value.exitstatus - end - - raise RemoteError, error unless exit_code.zero? - - tag_list.split("\n") - end end end end diff --git a/lib/gitlab/git_ref_validator.rb b/lib/gitlab/git_ref_validator.rb index 2e3e4fc3f1f..40636fb204e 100644 --- a/lib/gitlab/git_ref_validator.rb +++ b/lib/gitlab/git_ref_validator.rb @@ -7,11 +7,11 @@ module Gitlab # # Returns true for a valid reference name, false otherwise def validate(ref_name) - return false if ref_name.start_with?('refs/heads/') - return false if ref_name.start_with?('refs/remotes/') + not_allowed_prefixes = %w(refs/heads/ refs/remotes/ -) + return false if ref_name.start_with?(*not_allowed_prefixes) + return false if ref_name == 'HEAD' - Gitlab::Utils.system_silent( - %W(#{Gitlab.config.git.bin_path} check-ref-format --branch #{ref_name})) + Rugged::Reference.valid_name? "refs/heads/#{ref_name}" end end end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index 555733d1834..54c78fdb680 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -144,13 +144,14 @@ module Gitlab branch: encode_binary(target_branch) ) - branch_update = GitalyClient.call( + response = GitalyClient.call( @repository.storage, :operation_service, :user_ff_branch, request - ).branch_update - Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update) + ) + + Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) rescue GRPC::FailedPrecondition => e raise Gitlab::Git::CommitError, e end @@ -306,9 +307,9 @@ module Gitlab raise Gitlab::Git::CommitError, response.commit_error elsif response.create_tree_error.presence raise Gitlab::Git::Repository::CreateTreeError, response.create_tree_error - else - Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) end + + Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) end def user_commit_files_request_header( diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index 1ad6376dac7..fbe7d4ba1ae 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -17,6 +17,13 @@ module Gitlab consume_find_all_branches_response(response) end + def remote_branches(remote_name) + request = Gitaly::FindAllRemoteBranchesRequest.new(repository: @gitaly_repo, remote_name: remote_name) + response = GitalyClient.call(@repository.storage, :ref_service, :find_all_remote_branches, request) + + consume_find_all_remote_branches_response(remote_name, response) + end + def merged_branches(branch_names = []) request = Gitaly::FindAllBranchesRequest.new( repository: @gitaly_repo, @@ -243,6 +250,18 @@ module Gitlab end end + def consume_find_all_remote_branches_response(remote_name, response) + remote_name += '/' unless remote_name.ends_with?('/') + + response.flat_map do |message| + message.branches.map do |branch| + target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit) + branch_name = branch.name.sub(remote_name, '') + Gitlab::Git::Branch.new(@repository, branch_name, branch.target_commit.id, target_commit) + end + end + end + def consume_tags_response(response) response.flat_map do |message| message.tags.map { |gitaly_tag| Gitlab::Git::Tag.new(@repository, gitaly_tag) } diff --git a/lib/gitlab/profiler.rb b/lib/gitlab/profiler.rb index ecff6ab5d5e..c5bb4648572 100644 --- a/lib/gitlab/profiler.rb +++ b/lib/gitlab/profiler.rb @@ -146,5 +146,11 @@ module Gitlab logger.info("#{model} total (#{query_count}): #{time.round(2)}ms") end end + + def self.print_by_total_time(result, options = {}) + default_options = { sort_method: :total_time } + + Gitlab::Profiler::TotalTimeFlatPrinter.new(result).print(STDOUT, default_options.merge(options)) + end end end diff --git a/lib/gitlab/profiler/total_time_flat_printer.rb b/lib/gitlab/profiler/total_time_flat_printer.rb new file mode 100644 index 00000000000..2fd0ec10ba8 --- /dev/null +++ b/lib/gitlab/profiler/total_time_flat_printer.rb @@ -0,0 +1,39 @@ +module Gitlab + module Profiler + class TotalTimeFlatPrinter < RubyProf::FlatPrinter + def max_percent + @options[:max_percent] || 100 + end + + # Copied from: + # <https://github.com/ruby-prof/ruby-prof/blob/master/lib/ruby-prof/printers/flat_printer.rb> + # + # The changes are just to filter by total time, not self time, and add a + # max_percent option as well. + def print_methods(thread) + total_time = thread.total_time + methods = thread.methods.sort_by(&sort_method).reverse + + sum = 0 + methods.each do |method| + total_percent = (method.total_time / total_time) * 100 + next if total_percent < min_percent + next if total_percent > max_percent + + sum += method.self_time + + @output << "%6.2f %9.3f %9.3f %9.3f %9.3f %8d %s%s\n" % [ + method.self_time / total_time * 100, # %self + method.total_time, # total + method.self_time, # self + method.wait_time, # wait + method.children_time, # children + method.called, # calls + method.recursive? ? "*" : " ", # cycle + method_name(method) # name + ] + end + end + end + end +end diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb index de8b6ec69ce..308a95d2f09 100644 --- a/lib/gitlab/url_sanitizer.rb +++ b/lib/gitlab/url_sanitizer.rb @@ -71,12 +71,10 @@ module Gitlab def generate_full_url return @url unless valid_credentials? - @full_url = @url.dup - - @full_url.password = credentials[:password] if credentials[:password].present? - @full_url.user = credentials[:user] if credentials[:user].present? - - @full_url + @url.dup.tap do |generated| + generated.password = encode_percent(credentials[:password]) if credentials[:password].present? + generated.user = encode_percent(credentials[:user]) if credentials[:user].present? + end end def safe_url @@ -89,5 +87,10 @@ module Gitlab def valid_credentials? credentials && credentials.is_a?(Hash) && credentials.any? end + + def encode_percent(string) + # CGI.escape converts spaces to +, but this doesn't work for git clone + CGI.escape(string).gsub('+', '%20') + end end end diff --git a/lib/gitlab/user_activities.rb b/lib/gitlab/user_activities.rb deleted file mode 100644 index 125488536e1..00000000000 --- a/lib/gitlab/user_activities.rb +++ /dev/null @@ -1,34 +0,0 @@ -module Gitlab - class UserActivities - include Enumerable - - KEY = 'users:activities'.freeze - BATCH_SIZE = 500 - - def self.record(key, time = Time.now) - Gitlab::Redis::SharedState.with do |redis| - redis.hset(KEY, key, time.to_i) - end - end - - def delete(*keys) - Gitlab::Redis::SharedState.with do |redis| - redis.hdel(KEY, keys) - end - end - - def each - cursor = 0 - loop do - cursor, pairs = - Gitlab::Redis::SharedState.with do |redis| - redis.hscan(KEY, cursor, count: BATCH_SIZE) - end - - Hash[pairs].each { |pair| yield pair } - - break if cursor == '0' - end - end - end -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8ba05827682..19973f4f321 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1286,6 +1286,9 @@ msgstr "" msgid "ClusterIntegration|Helm Tiller" msgstr "" +msgid "ClusterIntegration|Hide" +msgstr "" + msgid "ClusterIntegration|Ingress" msgstr "" diff --git a/package.json b/package.json index 9dd7d80945f..e1801d4d435 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "webpack-prod": "NODE_ENV=production webpack --config config/webpack.config.js" }, "dependencies": { - "@gitlab-org/gitlab-svgs": "^1.25.0", + "@gitlab-org/gitlab-svgs": "^1.26.0", "autosize": "^4.0.0", "axios": "^0.17.1", "babel-core": "^6.26.3", diff --git a/scripts/lint-rugged b/scripts/lint-rugged index cabd083e9f9..d0c2c544c47 100755 --- a/scripts/lint-rugged +++ b/scripts/lint-rugged @@ -14,7 +14,10 @@ ALLOWED = [ 'lib/tasks/gitlab/cleanup.rake', # The only place where Rugged code is still allowed in production - 'lib/gitlab/git/' + 'lib/gitlab/git/', + + # Needed to avoid using the git binary to validate a branch name + 'lib/gitlab/git_ref_validator.rb' ].freeze rugged_lines = IO.popen(%w[git grep -i -n rugged -- app config lib], &:read).lines diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 74f362fd7fc..f1165c73847 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -89,6 +89,32 @@ describe ApplicationController do end end + describe 'session expiration' do + controller(described_class) do + def index + render text: 'authenticated' + end + end + + context 'authenticated user' do + it 'does not set the expire_after option' do + sign_in(create(:user)) + + get :index + + expect(request.env['rack.session.options'][:expire_after]).to be_nil + end + end + + context 'unauthenticated user' do + it 'sets the expire_after option' do + get :index + + expect(request.env['rack.session.options'][:expire_after]).to eq(Settings.gitlab['unauthenticated_session_expire_delay']) + end + end + end + describe 'rescue from Gitlab::Git::Storage::Inaccessible' do controller(described_class) do def index diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 7c00652317b..8e25b61e2f1 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -50,8 +50,6 @@ describe SessionsController do end context 'when using valid password', :clean_gitlab_redis_shared_state do - include UserActivitiesHelpers - let(:user) { create(:user) } let(:user_params) { { login: user.username, password: user.password } } @@ -77,7 +75,7 @@ describe SessionsController do it 'updates the user activity' do expect do post(:create, user: user_params) - end.to change { user_activity(user) } + end.to change { user.reload.last_activity_on }.to(Date.today) end end diff --git a/spec/features/user_sees_revert_modal_spec.rb b/spec/features/user_sees_revert_modal_spec.rb index 11a9e470f76..3b48ea4786d 100644 --- a/spec/features/user_sees_revert_modal_spec.rb +++ b/spec/features/user_sees_revert_modal_spec.rb @@ -9,6 +9,9 @@ describe 'Merge request > User sees revert modal', :js do sign_in(user) visit(project_merge_request_path(project, merge_request)) click_button('Merge') + + wait_for_requests + visit(merge_request_path(merge_request)) click_link('Revert') end diff --git a/spec/helpers/icons_helper_spec.rb b/spec/helpers/icons_helper_spec.rb index 93d8e672f8c..82f588d1a08 100644 --- a/spec/helpers/icons_helper_spec.rb +++ b/spec/helpers/icons_helper_spec.rb @@ -55,6 +55,29 @@ describe IconsHelper do expect(sprite_icon(icon_name, size: 72, css_class: 'icon-danger').to_s) .to eq "<svg class=\"s72 icon-danger\"><use xlink:href=\"#{icons_path}##{icon_name}\"></use></svg>" end + + describe 'non existing icon' do + non_existing = 'non_existing_icon_sprite' + + it 'should raise in development mode' do + allow(Rails.env).to receive(:development?).and_return(true) + + expect { sprite_icon(non_existing) }.to raise_error(ArgumentError, /is not a known icon/) + end + + it 'should raise in test mode' do + allow(Rails.env).to receive(:test?).and_return(true) + + expect { sprite_icon(non_existing) }.to raise_error(ArgumentError, /is not a known icon/) + end + + it 'should not raise in production mode' do + allow(Rails.env).to receive(:test?).and_return(false) + allow(Rails.env).to receive(:development?).and_return(false) + + expect { sprite_icon(non_existing) }.not_to raise_error + end + end end describe 'file_type_icon_class' do diff --git a/spec/javascripts/clusters/clusters_bundle_spec.js b/spec/javascripts/clusters/clusters_bundle_spec.js index abe2954d506..839b8a06b48 100644 --- a/spec/javascripts/clusters/clusters_bundle_spec.js +++ b/spec/javascripts/clusters/clusters_bundle_spec.js @@ -45,17 +45,33 @@ describe('Clusters', () => { }); describe('showToken', () => { - it('should update tye field type', () => { + it('should update token field type', () => { cluster.showTokenButton.click(); + expect( cluster.tokenField.getAttribute('type'), ).toEqual('text'); cluster.showTokenButton.click(); + expect( cluster.tokenField.getAttribute('type'), ).toEqual('password'); }); + + it('should update show token button text', () => { + cluster.showTokenButton.click(); + + expect( + cluster.showTokenButton.textContent, + ).toEqual('Hide'); + + cluster.showTokenButton.click(); + + expect( + cluster.showTokenButton.textContent, + ).toEqual('Show'); + }); }); describe('checkForNewInstalls', () => { diff --git a/spec/javascripts/helpers/vuex_action_helper.js b/spec/javascripts/helpers/vuex_action_helper.js index 4ca7015184e..dd9174194a1 100644 --- a/spec/javascripts/helpers/vuex_action_helper.js +++ b/spec/javascripts/helpers/vuex_action_helper.js @@ -84,14 +84,12 @@ export default ( done(); }; - return new Promise((resolve, reject) => { - try { - const result = action({ commit, state, dispatch, rootState: state }, payload); - resolve(result); - } catch (e) { - reject(e); - } + const result = action({ commit, state, dispatch, rootState: state }, payload); + + return new Promise(resolve => { + setImmediate(resolve); }) + .then(() => result) .catch(error => { validateResults(); throw error; diff --git a/spec/javascripts/helpers/vuex_action_helper_spec.js b/spec/javascripts/helpers/vuex_action_helper_spec.js index 8d6ad6750c0..09f0bd395c3 100644 --- a/spec/javascripts/helpers/vuex_action_helper_spec.js +++ b/spec/javascripts/helpers/vuex_action_helper_spec.js @@ -138,4 +138,29 @@ describe('VueX test helper (testAction)', () => { }); }); }); + + it('should work with async actions not returning promises', done => { + const data = { FOO: 'BAR' }; + + const promiseAction = ({ commit, dispatch }) => { + dispatch('ACTION'); + + axios + .get(TEST_HOST) + .then(() => { + commit('SUCCESS'); + return data; + }) + .catch(error => { + commit('ERROR'); + throw error; + }); + }; + + mock.onGet(TEST_HOST).replyOnce(200, 42); + + assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] }; + + testAction(promiseAction, null, {}, assertion.mutations, assertion.actions, done); + }); }); diff --git a/spec/javascripts/ide/components/ide_status_bar_spec.js b/spec/javascripts/ide/components/ide_status_bar_spec.js index 9895682f388..0e93c5193a1 100644 --- a/spec/javascripts/ide/components/ide_status_bar_spec.js +++ b/spec/javascripts/ide/components/ide_status_bar_spec.js @@ -70,7 +70,7 @@ describe('ideStatusBar', () => { status: { text: 'success', details_path: 'test', - icon: 'success', + icon: 'status_success', }, }, }); diff --git a/spec/javascripts/ide/components/jobs/detail/description_spec.js b/spec/javascripts/ide/components/jobs/detail/description_spec.js index 9b715a41499..babae00d2f7 100644 --- a/spec/javascripts/ide/components/jobs/detail/description_spec.js +++ b/spec/javascripts/ide/components/jobs/detail/description_spec.js @@ -23,6 +23,6 @@ describe('IDE job description', () => { }); it('renders CI icon', () => { - expect(vm.$el.querySelector('.ci-status-icon .ic-status_passed_borderless')).not.toBe(null); + expect(vm.$el.querySelector('.ci-status-icon .ic-status_success_borderless')).not.toBe(null); }); }); diff --git a/spec/javascripts/ide/components/jobs/item_spec.js b/spec/javascripts/ide/components/jobs/item_spec.js index 79e07f00e7b..2f97d39e98e 100644 --- a/spec/javascripts/ide/components/jobs/item_spec.js +++ b/spec/javascripts/ide/components/jobs/item_spec.js @@ -24,7 +24,7 @@ describe('IDE jobs item', () => { }); it('renders CI icon', () => { - expect(vm.$el.querySelector('.ic-status_passed_borderless')).not.toBe(null); + expect(vm.$el.querySelector('.ic-status_success_borderless')).not.toBe(null); }); it('does not render view logs button if not started', done => { diff --git a/spec/javascripts/ide/mock_data.js b/spec/javascripts/ide/mock_data.js index 4bbda4c8e80..7be450a0df7 100644 --- a/spec/javascripts/ide/mock_data.js +++ b/spec/javascripts/ide/mock_data.js @@ -74,7 +74,7 @@ export const jobs = [ name: 'test', path: 'testing', status: { - icon: 'status_passed', + icon: 'status_success', text: 'passed', }, stage: 'test', @@ -86,7 +86,7 @@ export const jobs = [ name: 'test 2', path: 'testing2', status: { - icon: 'status_passed', + icon: 'status_success', text: 'passed', }, stage: 'test', @@ -98,7 +98,7 @@ export const jobs = [ name: 'test 3', path: 'testing3', status: { - icon: 'status_passed', + icon: 'status_success', text: 'passed', }, stage: 'test', @@ -146,7 +146,7 @@ export const fullPipelinesResponse = { }, details: { status: { - icon: 'status_passed', + icon: 'status_success', text: 'passed', }, stages: [...stages], diff --git a/spec/javascripts/jobs/header_spec.js b/spec/javascripts/jobs/header_spec.js index cef30a007db..e21e2c6d6e3 100644 --- a/spec/javascripts/jobs/header_spec.js +++ b/spec/javascripts/jobs/header_spec.js @@ -20,7 +20,7 @@ describe('Job details header', () => { job: { status: { group: 'failed', - icon: 'ci-status-failed', + icon: 'status_failed', label: 'failed', text: 'failed', details_path: 'path', diff --git a/spec/javascripts/jobs/mock_data.js b/spec/javascripts/jobs/mock_data.js index dd025255bd1..8fdd9b309b7 100644 --- a/spec/javascripts/jobs/mock_data.js +++ b/spec/javascripts/jobs/mock_data.js @@ -14,7 +14,7 @@ export default { finished_at: threeWeeksAgo.toISOString(), queued: 9.54, status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -72,7 +72,7 @@ export default { }, details: { status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', diff --git a/spec/javascripts/pipelines/graph/job_component_spec.js b/spec/javascripts/pipelines/graph/job_component_spec.js index 9c55a19ebc7..56476253ad0 100644 --- a/spec/javascripts/pipelines/graph/job_component_spec.js +++ b/spec/javascripts/pipelines/graph/job_component_spec.js @@ -10,7 +10,7 @@ describe('pipeline graph job component', () => { id: 4256, name: 'test', status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', tooltip: 'passed', @@ -65,7 +65,7 @@ describe('pipeline graph job component', () => { id: 4257, name: 'test', status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -111,7 +111,7 @@ describe('pipeline graph job component', () => { id: 4258, name: 'test', status: { - icon: 'icon_status_success', + icon: 'status_success', }, }, }); @@ -125,7 +125,7 @@ describe('pipeline graph job component', () => { id: 4259, name: 'test', status: { - icon: 'icon_status_success', + icon: 'status_success', label: 'success', tooltip: 'success', }, diff --git a/spec/javascripts/pipelines/graph/job_name_component_spec.js b/spec/javascripts/pipelines/graph/job_name_component_spec.js index 8e2071ba0b3..c861d452dd0 100644 --- a/spec/javascripts/pipelines/graph/job_name_component_spec.js +++ b/spec/javascripts/pipelines/graph/job_name_component_spec.js @@ -10,7 +10,7 @@ describe('job name component', () => { propsData: { name: 'foo', status: { - icon: 'icon_status_success', + icon: 'status_success', }, }, }).$mount(); diff --git a/spec/javascripts/pipelines/graph/mock_data.js b/spec/javascripts/pipelines/graph/mock_data.js index 9e25a4b3fed..b2161d54bce 100644 --- a/spec/javascripts/pipelines/graph/mock_data.js +++ b/spec/javascripts/pipelines/graph/mock_data.js @@ -13,7 +13,7 @@ export default { path: '/root/ci-mock/pipelines/123', details: { status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -33,7 +33,7 @@ export default { name: 'test', size: 1, status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -58,7 +58,7 @@ export default { created_at: '2017-04-13T09:25:18.959Z', updated_at: '2017-04-13T09:25:23.118Z', status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -78,7 +78,7 @@ export default { }, ], status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -98,7 +98,7 @@ export default { name: 'deploy to production', size: 1, status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -123,7 +123,7 @@ export default { created_at: '2017-04-19T14:29:46.463Z', updated_at: '2017-04-19T14:30:27.498Z', status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -145,7 +145,7 @@ export default { name: 'deploy to staging', size: 1, status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -170,7 +170,7 @@ export default { created_at: '2017-04-18T16:32:08.420Z', updated_at: '2017-04-18T16:32:12.631Z', status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', @@ -190,7 +190,7 @@ export default { }, ], status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', diff --git a/spec/javascripts/pipelines/graph/stage_column_component_spec.js b/spec/javascripts/pipelines/graph/stage_column_component_spec.js index f744f1af5e6..9d1e71fd117 100644 --- a/spec/javascripts/pipelines/graph/stage_column_component_spec.js +++ b/spec/javascripts/pipelines/graph/stage_column_component_spec.js @@ -7,7 +7,7 @@ describe('stage column component', () => { id: 4250, name: 'test', status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', group: 'success', diff --git a/spec/javascripts/pipelines/header_component_spec.js b/spec/javascripts/pipelines/header_component_spec.js index cecc7ceb53d..034d3b4957d 100644 --- a/spec/javascripts/pipelines/header_component_spec.js +++ b/spec/javascripts/pipelines/header_component_spec.js @@ -18,7 +18,7 @@ describe('Pipeline details header', () => { details: { status: { group: 'failed', - icon: 'ci-status-failed', + icon: 'status_failed', label: 'failed', text: 'failed', details_path: 'path', diff --git a/spec/javascripts/pipelines/stage_spec.js b/spec/javascripts/pipelines/stage_spec.js index 16f6db39d6a..3f6789759ae 100644 --- a/spec/javascripts/pipelines/stage_spec.js +++ b/spec/javascripts/pipelines/stage_spec.js @@ -20,7 +20,7 @@ describe('Pipelines stage component', () => { stage: { status: { group: 'success', - icon: 'icon_status_success', + icon: 'status_success', title: 'success', }, dropdown_path: 'path.json', @@ -84,7 +84,7 @@ describe('Pipelines stage component', () => { component.stage = { status: { group: 'running', - icon: 'running', + icon: 'status_running', title: 'running', }, dropdown_path: 'bar.json', diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index c0b5a7d4455..7fd1a2350f7 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -76,7 +76,7 @@ export default { path: '/root/acets-app/pipelines/172', details: { status: { - icon: 'icon_status_success', + icon: 'status_success', favicon: 'favicon_status_success', text: 'passed', label: 'passed', @@ -91,7 +91,7 @@ export default { name: 'build', title: 'build: failed', status: { - icon: 'icon_status_failed', + icon: 'status_failed', favicon: 'favicon_status_failed', text: 'failed', label: 'failed', @@ -106,7 +106,7 @@ export default { name: 'review', title: 'review: skipped', status: { - icon: 'icon_status_skipped', + icon: 'status_skipped', favicon: 'favicon_status_skipped', text: 'skipped', label: 'skipped', diff --git a/spec/javascripts/vue_shared/components/ci_icon_spec.js b/spec/javascripts/vue_shared/components/ci_icon_spec.js index 423bc746a22..b59a7d7544f 100644 --- a/spec/javascripts/vue_shared/components/ci_icon_spec.js +++ b/spec/javascripts/vue_shared/components/ci_icon_spec.js @@ -13,7 +13,7 @@ describe('CI Icon component', () => { it('should render a span element with an svg', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_success', + icon: 'status_success', }, }); @@ -24,7 +24,7 @@ describe('CI Icon component', () => { it('should render a success status', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_success', + icon: 'status_success', group: 'success', }, }); @@ -35,7 +35,7 @@ describe('CI Icon component', () => { it('should render a failed status', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_failed', + icon: 'status_failed', group: 'failed', }, }); @@ -46,7 +46,7 @@ describe('CI Icon component', () => { it('should render success with warnings status', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_warning', + icon: 'status_warning', group: 'warning', }, }); @@ -57,7 +57,7 @@ describe('CI Icon component', () => { it('should render pending status', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_pending', + icon: 'status_pending', group: 'pending', }, }); @@ -68,7 +68,7 @@ describe('CI Icon component', () => { it('should render running status', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_running', + icon: 'status_running', group: 'running', }, }); @@ -79,7 +79,7 @@ describe('CI Icon component', () => { it('should render created status', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_created', + icon: 'status_created', group: 'created', }, }); @@ -90,7 +90,7 @@ describe('CI Icon component', () => { it('should render skipped status', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_skipped', + icon: 'status_skipped', group: 'skipped', }, }); @@ -101,7 +101,7 @@ describe('CI Icon component', () => { it('should render canceled status', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_canceled', + icon: 'status_canceled', group: 'canceled', }, }); @@ -112,7 +112,7 @@ describe('CI Icon component', () => { it('should render status for manual action', () => { vm = mountComponent(Component, { status: { - icon: 'icon_status_manual', + icon: 'status_manual', group: 'manual', }, }); diff --git a/spec/javascripts/vue_shared/components/header_ci_component_spec.js b/spec/javascripts/vue_shared/components/header_ci_component_spec.js index 65499a2d730..f17818c17c7 100644 --- a/spec/javascripts/vue_shared/components/header_ci_component_spec.js +++ b/spec/javascripts/vue_shared/components/header_ci_component_spec.js @@ -12,7 +12,7 @@ describe('Header CI Component', () => { props = { status: { group: 'failed', - icon: 'ci-status-failed', + icon: 'status_failed', label: 'failed', text: 'failed', details_path: 'path', diff --git a/spec/javascripts/vue_shared/components/icon_spec.js b/spec/javascripts/vue_shared/components/icon_spec.js index cc030e29d61..882420e602e 100644 --- a/spec/javascripts/vue_shared/components/icon_spec.js +++ b/spec/javascripts/vue_shared/components/icon_spec.js @@ -10,7 +10,7 @@ describe('Sprite Icon Component', function () { const IconComponent = Vue.extend(Icon); icon = mountComponent(IconComponent, { - name: 'test', + name: 'commit', size: 32, cssClasses: 'extraclasses', }); @@ -30,7 +30,7 @@ describe('Sprite Icon Component', function () { it('should have <use> as a child element with the correct href', function () { expect(icon.$el.firstChild.tagName).toBe('use'); - expect(icon.$el.firstChild.getAttribute('xlink:href')).toBe(`${gon.sprite_icons}#test`); + expect(icon.$el.firstChild.getAttribute('xlink:href')).toBe(`${gon.sprite_icons}#commit`); }); it('should properly compute iconSizeClass', function () { @@ -50,5 +50,13 @@ describe('Sprite Icon Component', function () { expect(containsSizeClass).toBe(true); expect(containsCustomClass).toBe(true); }); + + it('`name` validator should return false for non existing icons', () => { + expect(Icon.props.name.validator('non_existing_icon_sprite')).toBe(false); + }); + + it('`name` validator should return false for existing icons', () => { + expect(Icon.props.name.validator('commit')).toBe(true); + }); }); }); diff --git a/spec/javascripts/vue_shared/components/notes/system_note_spec.js b/spec/javascripts/vue_shared/components/notes/system_note_spec.js index aa4c9c4c88c..2a6015fe35f 100644 --- a/spec/javascripts/vue_shared/components/notes/system_note_spec.js +++ b/spec/javascripts/vue_shared/components/notes/system_note_spec.js @@ -19,7 +19,7 @@ describe('system note component', () => { path: '/root', }, note_html: '<p dir="auto">closed</p>', - system_note_icon_name: 'icon_status_closed', + system_note_icon_name: 'status_closed', created_at: '2017-08-02T10:51:58.559Z', }, }; diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 0365c3b20ef..ee385226e65 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -602,40 +602,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#remote_tags' do - let(:remote_name) { 'upstream' } - let(:target_commit_id) { SeedRepo::Commit::ID } - let(:tag_name) { 'v0.0.1' } - let(:tag_message) { 'My tag' } - let(:remote_repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end - - around do |example| - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - subject { repository.remote_tags(remote_name) } - - before do - remote_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { remote_repository.path } - repository.add_remote(remote_name, remote_repository_path) - remote_repository.add_tag(tag_name, user: user, target: target_commit_id) - end - - after do - ensure_seeds - end - - it 'gets the remote tags' do - expect(subject.first).to be_an_instance_of(Gitlab::Git::Tag) - expect(subject.first.name).to eq(tag_name) - expect(subject.first.dereferenced_target.id).to eq(target_commit_id) - end - end - describe "#log" do shared_examples 'repository log' do let(:commit_with_old_name) do diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 031d1e87dc1..eaf64e3c9b4 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' describe Gitlab::GitalyClient::OperationService do - let(:project) { create(:project) } + set(:project) { create(:project, :repository) } let(:repository) { project.repository.raw } let(:client) { described_class.new(repository) } - let(:user) { create(:user) } + set(:user) { create(:user) } let(:gitaly_user) { Gitlab::Git::User.from_gitlab(user).to_gitaly } describe '#user_create_branch' do @@ -151,18 +151,104 @@ describe Gitlab::GitalyClient::OperationService do end let(:response) { Gitaly::UserFFBranchResponse.new(branch_update: branch_update) } - subject { client.user_ff_branch(user, source_sha, target_branch) } - - it 'sends a user_ff_branch message and returns a BranchUpdate object' do + before do expect_any_instance_of(Gitaly::OperationService::Stub) .to receive(:user_ff_branch).with(request, kind_of(Hash)) .and_return(response) + end + subject { client.user_ff_branch(user, source_sha, target_branch) } + + it 'sends a user_ff_branch message and returns a BranchUpdate object' do expect(subject).to be_a(Gitlab::Git::OperationService::BranchUpdate) expect(subject.newrev).to eq(source_sha) expect(subject.repo_created).to be(false) expect(subject.branch_created).to be(false) end + + context 'when the response has no branch_update' do + let(:response) { Gitaly::UserFFBranchResponse.new } + + it { expect(subject).to be_nil } + end + end + + shared_examples 'cherry pick and revert errors' do + context 'when a pre_receive_error is present' do + let(:response) { response_class.new(pre_receive_error: "something failed") } + + it 'raises a PreReceiveError' do + expect { subject }.to raise_error(Gitlab::Git::PreReceiveError, "something failed") + end + end + + context 'when a commit_error is present' do + let(:response) { response_class.new(commit_error: "something failed") } + + it 'raises a CommitError' do + expect { subject }.to raise_error(Gitlab::Git::CommitError, "something failed") + end + end + + context 'when a create_tree_error is present' do + let(:response) { response_class.new(create_tree_error: "something failed") } + + it 'raises a CreateTreeError' do + expect { subject }.to raise_error(Gitlab::Git::Repository::CreateTreeError, "something failed") + end + end + + context 'when branch_update is nil' do + let(:response) { response_class.new } + + it { expect(subject).to be_nil } + end + end + + describe '#user_cherry_pick' do + let(:response_class) { Gitaly::UserCherryPickResponse } + + subject do + client.user_cherry_pick( + user: user, + commit: repository.commit, + branch_name: 'master', + message: 'Cherry-pick message', + start_branch_name: 'master', + start_repository: repository + ) + end + + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_cherry_pick).with(kind_of(Gitaly::UserCherryPickRequest), kind_of(Hash)) + .and_return(response) + end + + it_behaves_like 'cherry pick and revert errors' + end + + describe '#user_revert' do + let(:response_class) { Gitaly::UserRevertResponse } + + subject do + client.user_revert( + user: user, + commit: repository.commit, + branch_name: 'master', + message: 'Revert message', + start_branch_name: 'master', + start_repository: repository + ) + end + + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_revert).with(kind_of(Gitaly::UserRevertRequest), kind_of(Hash)) + .and_return(response) + end + + it_behaves_like 'cherry pick and revert errors' end describe '#user_squash' do @@ -203,7 +289,7 @@ describe Gitlab::GitalyClient::OperationService do Gitaly::UserSquashResponse.new(git_error: "something failed") end - it "throws a PreReceive exception" do + it "raises a GitError exception" do expect_any_instance_of(Gitaly::OperationService::Stub) .to receive(:user_squash).with(request, kind_of(Hash)) .and_return(response) @@ -212,5 +298,41 @@ describe Gitlab::GitalyClient::OperationService do Gitlab::Git::Repository::GitError, "something failed") end end + + describe '#user_commit_files' do + subject do + client.user_commit_files( + gitaly_user, 'my-branch', 'Commit files message', [], 'janedoe@example.com', 'Jane Doe', + 'master', repository) + end + + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_commit_files).with(kind_of(Enumerator), kind_of(Hash)) + .and_return(response) + end + + context 'when a pre_receive_error is present' do + let(:response) { Gitaly::UserCommitFilesResponse.new(pre_receive_error: "something failed") } + + it 'raises a PreReceiveError' do + expect { subject }.to raise_error(Gitlab::Git::PreReceiveError, "something failed") + end + end + + context 'when an index_error is present' do + let(:response) { Gitaly::UserCommitFilesResponse.new(index_error: "something failed") } + + it 'raises a PreReceiveError' do + expect { subject }.to raise_error(Gitlab::Git::Index::IndexError, "something failed") + end + end + + context 'when branch_update is nil' do + let(:response) { Gitaly::UserCommitFilesResponse.new } + + it { expect(subject).to be_nil } + end + end end end diff --git a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb index 257e4c50f2d..400d426c949 100644 --- a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb @@ -18,6 +18,44 @@ describe Gitlab::GitalyClient::RefService do end end + describe '#remote_branches' do + let(:remote_name) { 'my_remote' } + subject { client.remote_branches(remote_name) } + + it 'sends a find_all_remote_branches message' do + expect_any_instance_of(Gitaly::RefService::Stub) + .to receive(:find_all_remote_branches) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return([]) + + subject + end + + it 'concantes and returns the response branches as Gitlab::Git::Branch objects' do + target_commits = create_list(:gitaly_commit, 4) + response_branches = target_commits.each_with_index.map do |gitaly_commit, i| + Gitaly::Branch.new(name: "#{remote_name}/#{i}", target_commit: gitaly_commit) + end + response = [ + Gitaly::FindAllRemoteBranchesResponse.new(branches: response_branches[0, 2]), + Gitaly::FindAllRemoteBranchesResponse.new(branches: response_branches[2, 2]) + ] + + expect_any_instance_of(Gitaly::RefService::Stub) + .to receive(:find_all_remote_branches).and_return(response) + + expect(subject.length).to be(response_branches.length) + + response_branches.each_with_index do |gitaly_branch, i| + branch = subject[i] + commit = Gitlab::Git::Commit.new(repository, gitaly_branch.target_commit) + + expect(branch.name).to eq(i.to_s) # It removes the `remote/` prefix + expect(branch.dereferenced_target).to eq(commit) + end + end + end + describe '#branch_names' do it 'sends a find_all_branch_names message' do expect_any_instance_of(Gitaly::RefService::Stub) diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index 758a9bc5a2b..b41a81a8167 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -145,6 +145,10 @@ describe Gitlab::UrlSanitizer do 'http://foo:@example.com' | 'http://foo@example.com' 'http://:bar@example.com' | :same 'http://foo:bar@example.com' | :same + 'http://foo:g p@example.com' | 'http://foo:g%20p@example.com' + 'http://foo:s/h@example.com' | 'http://foo:s%2Fh@example.com' + 'http://t u:a#b@example.com' | 'http://t%20u:a%23b@example.com' + 'http://t+u:a#b@example.com' | 'http://t%2Bu:a%23b@example.com' end with_them do @@ -160,7 +164,7 @@ describe Gitlab::UrlSanitizer do url_sanitizer = described_class.new("https://foo:b?r@github.com/me/project.git") expect(url_sanitizer.sanitized_url).to eq("https://github.com/me/project.git") - expect(url_sanitizer.full_url).to eq("https://foo:b?r@github.com/me/project.git") + expect(url_sanitizer.full_url).to eq("https://foo:b%3Fr@github.com/me/project.git") end end end diff --git a/spec/lib/gitlab/user_activities_spec.rb b/spec/lib/gitlab/user_activities_spec.rb deleted file mode 100644 index 6bce2ee13cf..00000000000 --- a/spec/lib/gitlab/user_activities_spec.rb +++ /dev/null @@ -1,127 +0,0 @@ -require 'spec_helper' - -describe Gitlab::UserActivities, :clean_gitlab_redis_shared_state do - let(:now) { Time.now } - - describe '.record' do - context 'with no time given' do - it 'uses Time.now and records an activity in SharedState' do - Timecop.freeze do - now # eager-load now - described_class.record(42) - end - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]]) - end - end - end - - context 'with a time given' do - it 'uses the given time and records an activity in SharedState' do - described_class.record(42, now) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]]) - end - end - end - end - - describe '.delete' do - context 'with a single key' do - context 'and key exists' do - it 'removes the pair from SharedState' do - described_class.record(42, now) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]]) - end - - subject.delete(42) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []]) - end - end - end - - context 'and key does not exist' do - it 'removes the pair from SharedState' do - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []]) - end - - subject.delete(42) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []]) - end - end - end - end - - context 'with multiple keys' do - context 'and all keys exist' do - it 'removes the pair from SharedState' do - described_class.record(41, now) - described_class.record(42, now) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['41', now.to_i.to_s], ['42', now.to_i.to_s]]]) - end - - subject.delete(41, 42) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []]) - end - end - end - - context 'and some keys does not exist' do - it 'removes the existing pair from SharedState' do - described_class.record(42, now) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]]) - end - - subject.delete(41, 42) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []]) - end - end - end - end - end - - describe 'Enumerable' do - before do - described_class.record(40, now) - described_class.record(41, now) - described_class.record(42, now) - end - - it 'allows to read the activities sequentially' do - expected = { '40' => now.to_i.to_s, '41' => now.to_i.to_s, '42' => now.to_i.to_s } - - actual = described_class.new.each_with_object({}) do |(key, time), actual| - actual[key] = time - end - - expect(actual).to eq(expected) - end - - context 'with many records' do - before do - 1_000.times { |i| described_class.record(i, now) } - end - - it 'is possible to loop through all the records' do - expect(described_class.new.count).to eq(1_000) - end - end - end -end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index e65214808e1..5d64602ca56 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2225,20 +2225,6 @@ describe Repository do end end - describe '#remote_branches' do - it 'returns the remote branches' do - masterrev = repository.find_branch('master').dereferenced_target - create_remote_branch('joe', 'remote_branch', masterrev) - repository.add_branch(user, 'local_branch', masterrev.id) - - # TODO: move this test to gitaly https://gitlab.com/gitlab-org/gitaly/issues/1243 - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - expect(repository.remote_branches('joe').any? { |branch| branch.name == 'local_branch' }).to eq(false) - expect(repository.remote_branches('joe').any? { |branch| branch.name == 'remote_branch' }).to eq(true) - end - end - end - describe '#commit_count' do context 'with a non-existing repository' do it 'returns 0' do diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index a56b913198c..a2cfa706f58 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -279,7 +279,7 @@ describe API::Internal do expect(json_response["status"]).to be_truthy expect(json_response["repository_path"]).to eq('/') expect(json_response["gl_repository"]).to eq("wiki-#{project.id}") - expect(user).not_to have_an_activity_record + expect(user.reload.last_activity_on).to be_nil end end @@ -291,7 +291,7 @@ describe API::Internal do expect(json_response["status"]).to be_truthy expect(json_response["repository_path"]).to eq('/') expect(json_response["gl_repository"]).to eq("wiki-#{project.id}") - expect(user).to have_an_activity_record + expect(user.reload.last_activity_on).to eql(Date.today) end end @@ -309,7 +309,7 @@ describe API::Internal do expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path) expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage)) expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage)) - expect(user).to have_an_activity_record + expect(user.reload.last_activity_on).to eql(Date.today) end end @@ -328,7 +328,7 @@ describe API::Internal do expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path) expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage)) expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage)) - expect(user).not_to have_an_activity_record + expect(user.reload.last_activity_on).to be_nil end end end @@ -345,7 +345,7 @@ describe API::Internal do expect(response).to have_gitlab_http_status(200) expect(json_response["status"]).to be_falsey - expect(user).not_to have_an_activity_record + expect(user.reload.last_activity_on).to be_nil end end @@ -355,7 +355,7 @@ describe API::Internal do expect(response).to have_gitlab_http_status(200) expect(json_response["status"]).to be_falsey - expect(user).not_to have_an_activity_record + expect(user.reload.last_activity_on).to be_nil end end end @@ -373,7 +373,7 @@ describe API::Internal do expect(response).to have_gitlab_http_status(200) expect(json_response["status"]).to be_falsey - expect(user).not_to have_an_activity_record + expect(user.reload.last_activity_on).to be_nil end end @@ -383,7 +383,7 @@ describe API::Internal do expect(response).to have_gitlab_http_status(200) expect(json_response["status"]).to be_falsey - expect(user).not_to have_an_activity_record + expect(user.reload.last_activity_on).to be_nil end end end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index b030d9862c6..0f3e7157e14 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -5,7 +5,6 @@ describe 'Git HTTP requests' do include TermsHelper include GitHttpHelpers include WorkhorseHelpers - include UserActivitiesHelpers shared_examples 'pulls require Basic HTTP Authentication' do context "when no credentials are provided" do @@ -440,10 +439,10 @@ describe 'Git HTTP requests' do end it 'updates the user last activity', :clean_gitlab_redis_shared_state do - expect(user_activity(user)).to be_nil + expect(user.last_activity_on).to be_nil download(path, env) do |response| - expect(user_activity(user)).to be_present + expect(user.reload.last_activity_on).to eql(Date.today) end end end diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index 13395a7cac3..68e310b0506 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' describe EventCreateService do - include UserActivitiesHelpers - let(:service) { described_class.new } describe 'Issues' do @@ -146,7 +144,7 @@ describe EventCreateService do it 'updates user last activity' do expect { service.push(project, user, push_data) } - .to change { user_activity(user) } + .to change { user.last_activity_on }.to(Date.today) end it 'caches the last push event for the user' do diff --git a/spec/services/users/activity_service_spec.rb b/spec/services/users/activity_service_spec.rb index 17eabad73be..f20849e6924 100644 --- a/spec/services/users/activity_service_spec.rb +++ b/spec/services/users/activity_service_spec.rb @@ -1,60 +1,61 @@ require 'spec_helper' describe Users::ActivityService do - include UserActivitiesHelpers + include ExclusiveLeaseHelpers - let(:user) { create(:user) } + let(:user) { create(:user, last_activity_on: last_activity_on) } - subject(:service) { described_class.new(user, 'type') } + subject { described_class.new(user, 'type') } describe '#execute', :clean_gitlab_redis_shared_state do context 'when last activity is nil' do - before do - service.execute - end + let(:last_activity_on) { nil } - it 'sets the last activity timestamp for the user' do - expect(last_hour_user_ids).to eq([user.id]) + it 'updates last_activity_on for the user' do + expect { subject.execute } + .to change(user, :last_activity_on).from(last_activity_on).to(Date.today) end + end - it 'updates the same user' do - service.execute + context 'when last activity is in the past' do + let(:last_activity_on) { Date.today - 1.week } - expect(last_hour_user_ids).to eq([user.id]) - end - - it 'updates the timestamp of an existing user' do - Timecop.freeze(Date.tomorrow) do - expect { service.execute }.to change { user_activity(user) }.to(Time.now.to_i.to_s) - end + it 'updates last_activity_on for the user' do + expect { subject.execute } + .to change(user, :last_activity_on) + .from(last_activity_on) + .to(Date.today) end + end - describe 'other user' do - it 'updates other user' do - other_user = create(:user) - described_class.new(other_user, 'type').execute + context 'when last activity is today' do + let(:last_activity_on) { Date.today } - expect(last_hour_user_ids).to match_array([user.id, other_user.id]) - end + it 'does not update last_activity_on' do + expect { subject.execute }.not_to change(user, :last_activity_on) end end context 'when in GitLab read-only instance' do + let(:last_activity_on) { nil } + before do allow(Gitlab::Database).to receive(:read_only?).and_return(true) end - it 'does not update last_activity_at' do - service.execute - - expect(last_hour_user_ids).to eq([]) + it 'does not update last_activity_on' do + expect { subject.execute }.not_to change(user, :last_activity_on) end end - end - def last_hour_user_ids - Gitlab::UserActivities.new - .select { |k, v| v >= 1.hour.ago.to_i.to_s } - .map { |k, _| k.to_i } + context 'when a lease could not be obtained' do + let(:last_activity_on) { nil } + + it 'does not update last_activity_on' do + stub_exclusive_lease_taken("acitvity_service:#{user.id}", timeout: 1.minute.to_i) + + expect { subject.execute }.not_to change(user, :last_activity_on) + end + end end end diff --git a/spec/support/helpers/user_activities_helpers.rb b/spec/support/helpers/user_activities_helpers.rb deleted file mode 100644 index 44feb104644..00000000000 --- a/spec/support/helpers/user_activities_helpers.rb +++ /dev/null @@ -1,7 +0,0 @@ -module UserActivitiesHelpers - def user_activity(user) - Gitlab::UserActivities.new - .find { |k, _| k == user.id.to_s }&. - second - end -end diff --git a/spec/support/matchers/user_activity_matchers.rb b/spec/support/matchers/user_activity_matchers.rb deleted file mode 100644 index ce3b683b6d2..00000000000 --- a/spec/support/matchers/user_activity_matchers.rb +++ /dev/null @@ -1,5 +0,0 @@ -RSpec::Matchers.define :have_an_activity_record do |expected| - match do |user| - expect(Gitlab::UserActivities.new.find { |k, _| k == user.id.to_s }).to be_present - end -end diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb index 318aad4bc1e..f17c5ac6aac 100644 --- a/spec/workers/emails_on_push_worker_spec.rb +++ b/spec/workers/emails_on_push_worker_spec.rb @@ -100,10 +100,6 @@ describe EmailsOnPushWorker, :mailer do end context "when there are multiple recipients" do - let(:recipients) do - 1.upto(5).map { |i| user.email.sub('@', "+#{i}@") }.join("\n") - end - before do # This is a hack because we modify the mail object before sending, for efficency, # but the TestMailer adapter just appends the objects to an array. To clone a mail @@ -114,16 +110,57 @@ describe EmailsOnPushWorker, :mailer do end end - it "sends the mail to each of the recipients" do - perform - expect(ActionMailer::Base.deliveries.count).to eq(5) - expect(ActionMailer::Base.deliveries.map(&:to).flatten).to contain_exactly(*recipients.split) + context "when the recipient addresses are a list of email addresses" do + let(:recipients) do + 1.upto(5).map { |i| user.email.sub('@', "+#{i}@") }.join("\n") + end + + it "sends the mail to each of the recipients" do + perform + + expect(ActionMailer::Base.deliveries.count).to eq(5) + expect(email_recipients).to contain_exactly(*recipients.split) + end + + it "only generates the mail once" do + expect(Notify).to receive(:repository_push_email).once.and_call_original + expect(Premailer::Rails::CustomizedPremailer).to receive(:new).once.and_call_original + + perform + end end - it "only generates the mail once" do - expect(Notify).to receive(:repository_push_email).once.and_call_original - expect(Premailer::Rails::CustomizedPremailer).to receive(:new).once.and_call_original - perform + context "when the recipient addresses contains angle brackets and are separated by spaces" do + let(:recipients) { "John Doe <johndoe@example.com> Jane Doe <janedoe@example.com>" } + + it "accepts emails separated by whitespace" do + perform + + expect(ActionMailer::Base.deliveries.count).to eq(2) + expect(email_recipients).to contain_exactly("johndoe@example.com", "janedoe@example.com") + end + end + + context "when the recipient addresses contain a mix of emails with and without angle brackets" do + let(:recipients) { "johndoe@example.com Jane Doe <janedoe@example.com>" } + + it "accepts both kind of emails" do + perform + + expect(ActionMailer::Base.deliveries.count).to eq(2) + expect(email_recipients).to contain_exactly("johndoe@example.com", "janedoe@example.com") + end + end + + context "when the recipient addresses contains angle brackets and are separated by newlines" do + let(:recipients) { "John Doe <johndoe@example.com>\nJane Doe <janedoe@example.com>" } + + it "accepts emails separated by newlines" do + perform + + expect(ActionMailer::Base.deliveries.count).to eq(2) + expect(email_recipients).to contain_exactly("johndoe@example.com", "janedoe@example.com") + end end end end diff --git a/spec/workers/schedule_update_user_activity_worker_spec.rb b/spec/workers/schedule_update_user_activity_worker_spec.rb deleted file mode 100644 index 32c59381b01..00000000000 --- a/spec/workers/schedule_update_user_activity_worker_spec.rb +++ /dev/null @@ -1,25 +0,0 @@ -require 'spec_helper' - -describe ScheduleUpdateUserActivityWorker, :clean_gitlab_redis_shared_state do - let(:now) { Time.now } - - before do - Gitlab::UserActivities.record('1', now) - Gitlab::UserActivities.record('2', now) - end - - it 'schedules UpdateUserActivityWorker once' do - expect(UpdateUserActivityWorker).to receive(:perform_async).with({ '1' => now.to_i.to_s, '2' => now.to_i.to_s }) - - subject.perform - end - - context 'when specifying a batch size' do - it 'schedules UpdateUserActivityWorker twice' do - expect(UpdateUserActivityWorker).to receive(:perform_async).with({ '1' => now.to_i.to_s }) - expect(UpdateUserActivityWorker).to receive(:perform_async).with({ '2' => now.to_i.to_s }) - - subject.perform(1) - end - end -end diff --git a/spec/workers/update_user_activity_worker_spec.rb b/spec/workers/update_user_activity_worker_spec.rb deleted file mode 100644 index 268ca1d81f2..00000000000 --- a/spec/workers/update_user_activity_worker_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'spec_helper' - -describe UpdateUserActivityWorker, :clean_gitlab_redis_shared_state do - let(:user_active_2_days_ago) { create(:user, current_sign_in_at: 10.months.ago) } - let(:user_active_yesterday_1) { create(:user) } - let(:user_active_yesterday_2) { create(:user) } - let(:user_active_today) { create(:user) } - let(:data) do - { - user_active_2_days_ago.id.to_s => 2.days.ago.at_midday.to_i.to_s, - user_active_yesterday_1.id.to_s => 1.day.ago.at_midday.to_i.to_s, - user_active_yesterday_2.id.to_s => 1.day.ago.at_midday.to_i.to_s, - user_active_today.id.to_s => Time.now.to_i.to_s - } - end - - it 'updates users.last_activity_on' do - subject.perform(data) - - aggregate_failures do - expect(user_active_2_days_ago.reload.last_activity_on).to eq(2.days.ago.to_date) - expect(user_active_yesterday_1.reload.last_activity_on).to eq(1.day.ago.to_date) - expect(user_active_yesterday_2.reload.last_activity_on).to eq(1.day.ago.to_date) - expect(user_active_today.reload.reload.last_activity_on).to eq(Date.today) - end - end - - it 'deletes the pairs from SharedState' do - data.each { |id, time| Gitlab::UserActivities.record(id, time) } - - subject.perform(data) - - expect(Gitlab::UserActivities.new.to_a).to be_empty - end -end diff --git a/yarn.lock b/yarn.lock index 9d0eae5f509..67f9aa98c74 100644 --- a/yarn.lock +++ b/yarn.lock @@ -78,9 +78,9 @@ lodash "^4.2.0" to-fast-properties "^2.0.0" -"@gitlab-org/gitlab-svgs@^1.25.0": - version "1.25.0" - resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-svgs/-/gitlab-svgs-1.25.0.tgz#1a82b1be43e1a46e6b0767ef46f26f5fd6bbd101" +"@gitlab-org/gitlab-svgs@^1.26.0": + version "1.26.0" + resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-svgs/-/gitlab-svgs-1.26.0.tgz#d89c633e866d031a9e4787b05eacc7144c3a7791" "@sindresorhus/is@^0.7.0": version "0.7.0" |