Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2018-07-20 08:24:37 +0300
committerStan Hu <stanhu@gmail.com>2018-07-20 08:24:37 +0300
commit81b5611efb40821783eb000f312f91ccce3bcadf (patch)
tree6cc247db2dc0bdf391f6b8a3524fbe903d2df1d3
parent2b9ec17e040863eb37255fa89f03fabdcd113cce (diff)
parentb60364c0f377f4afdf5e96e84aba11988f4ae526 (diff)
Merge branch 'master' into sh-support-bitbucket-server-import
-rw-r--r--app/models/user.rb1
-rw-r--r--app/services/prometheus/adapter_service.rb2
-rw-r--r--app/services/protected_branches/access_level_params.rb2
-rw-r--r--app/services/protected_branches/api_service.rb2
-rw-r--r--app/services/protected_branches/create_service.rb2
-rw-r--r--app/services/protected_branches/destroy_service.rb2
-rw-r--r--app/services/protected_branches/legacy_api_create_service.rb2
-rw-r--r--app/services/protected_branches/legacy_api_update_service.rb2
-rw-r--r--app/services/protected_branches/update_service.rb2
-rw-r--r--app/services/protected_tags/create_service.rb2
-rw-r--r--app/services/protected_tags/destroy_service.rb2
-rw-r--r--app/services/protected_tags/update_service.rb2
-rw-r--r--app/services/quick_actions/interpret_service.rb2
-rw-r--r--app/services/search/global_service.rb2
-rw-r--r--app/services/search/group_service.rb2
-rw-r--r--app/services/search/project_service.rb2
-rw-r--r--app/services/search/snippet_service.rb2
-rw-r--r--app/services/tags/create_service.rb2
-rw-r--r--app/services/tags/destroy_service.rb2
-rw-r--r--app/services/test_hooks/base_service.rb2
-rw-r--r--app/services/test_hooks/project_service.rb2
-rw-r--r--app/services/test_hooks/system_service.rb2
-rw-r--r--app/services/users/activity_service.rb22
-rw-r--r--app/services/users/build_service.rb2
-rw-r--r--app/services/users/create_service.rb2
-rw-r--r--app/services/users/destroy_service.rb2
-rw-r--r--app/services/users/last_push_event_service.rb2
-rw-r--r--app/services/users/migrate_to_ghost_user_service.rb2
-rw-r--r--app/services/users/refresh_authorized_projects_service.rb2
-rw-r--r--app/services/users/respond_to_terms_service.rb2
-rw-r--r--app/services/users/update_service.rb2
-rw-r--r--app/services/wiki_pages/base_service.rb2
-rw-r--r--app/services/wiki_pages/create_service.rb2
-rw-r--r--app/services/wiki_pages/destroy_service.rb2
-rw-r--r--app/services/wiki_pages/update_service.rb2
-rw-r--r--app/views/profiles/two_factor_auths/show.html.haml8
-rw-r--r--app/workers/all_queues.yml2
-rw-r--r--app/workers/schedule_update_user_activity_worker.rb12
-rw-r--r--app/workers/update_user_activity_worker.rb27
-rw-r--r--changelogs/unreleased/43312-remove_user_activity_workers.yml5
-rw-r--r--changelogs/unreleased/add-total-time-flat-printer-for-profiling.yml6
-rw-r--r--changelogs/unreleased/frozen-string-enable-apps-services-inner-even-more.yml5
-rw-r--r--changelogs/unreleased/gitaly-ff-branch-nil.yml5
-rw-r--r--changelogs/unreleased/rails5-fix-revert-modal-spec.yml5
-rw-r--r--changelogs/unreleased/regen-2fa-codes.yml5
-rw-r--r--config/initializers/1_settings.rb4
-rw-r--r--config/sidekiq_queues.yml2
-rw-r--r--doc/api/projects.md5
-rw-r--r--doc/development/profiling.md30
-rw-r--r--lib/api/projects.rb21
-rw-r--r--lib/gitlab/git/operation_service.rb2
-rw-r--r--lib/gitlab/git_ref_validator.rb8
-rw-r--r--lib/gitlab/gitaly_client/operation_service.rb11
-rw-r--r--lib/gitlab/profiler.rb6
-rw-r--r--lib/gitlab/profiler/total_time_flat_printer.rb39
-rw-r--r--lib/gitlab/user_activities.rb34
-rwxr-xr-xscripts/lint-rugged5
-rw-r--r--spec/controllers/sessions_controller_spec.rb4
-rw-r--r--spec/features/user_sees_revert_modal_spec.rb3
-rw-r--r--spec/javascripts/helpers/vuex_action_helper.js12
-rw-r--r--spec/javascripts/helpers/vuex_action_helper_spec.js25
-rw-r--r--spec/lib/gitlab/gitaly_client/operation_service_spec.rb134
-rw-r--r--spec/lib/gitlab/user_activities_spec.rb127
-rw-r--r--spec/requests/api/internal_spec.rb16
-rw-r--r--spec/requests/git_http_spec.rb5
-rw-r--r--spec/services/event_create_service_spec.rb4
-rw-r--r--spec/services/users/activity_service_spec.rb65
-rw-r--r--spec/support/helpers/user_activities_helpers.rb7
-rw-r--r--spec/support/matchers/user_activity_matchers.rb5
-rw-r--r--spec/workers/schedule_update_user_activity_worker_spec.rb25
-rw-r--r--spec/workers/update_user_activity_worker_spec.rb35
71 files changed, 442 insertions, 361 deletions
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/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/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/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 44bc72a7185..c3122827a6b 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -319,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/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_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/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/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/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/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/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/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/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/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/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