diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-09-11 09:40:59 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-09-11 09:40:59 +0000 |
commit | 5cc140b202a0fc9bf2056b481c977bbcf9255919 (patch) | |
tree | 6addace4503068db70271bb3fe1e5993ff3f263b /app | |
parent | 2071671cd0deb39ab29dda7d04e48a649c3643cb (diff) | |
parent | 83355336dd405a3af9914a6ae6d397a631847e49 (diff) | |
download | gitlab-ce-5cc140b202a0fc9bf2056b481c977bbcf9255919.tar.gz |
Merge branch 'user-recent-push' into 'master'
Rework how recent push events are retrieved
Closes #35990
See merge request !13995
Diffstat (limited to 'app')
-rw-r--r-- | app/helpers/projects_helper.rb | 10 | ||||
-rw-r--r-- | app/models/event.rb | 2 | ||||
-rw-r--r-- | app/models/push_event.rb | 38 | ||||
-rw-r--r-- | app/models/user.rb | 19 | ||||
-rw-r--r-- | app/services/event_create_service.rb | 13 | ||||
-rw-r--r-- | app/services/users/last_push_event_service.rb | 83 |
6 files changed, 139 insertions, 26 deletions
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index c0114dd0256..0c8cb9ba235 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -137,15 +137,7 @@ module ProjectsHelper end def last_push_event - return unless current_user - return current_user.recent_push unless @project - - project_ids = [@project.id] - if fork = current_user.fork_of(@project) - project_ids << fork.id - end - - current_user.recent_push(project_ids) + current_user&.recent_push(@project) end def project_feature_access_select(field) diff --git a/app/models/event.rb b/app/models/event.rb index c313bbb66f8..8e9490b66f4 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -49,7 +49,7 @@ class Event < ActiveRecord::Base belongs_to :author, class_name: "User" belongs_to :project belongs_to :target, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations - has_one :push_event_payload, foreign_key: :event_id + has_one :push_event_payload # Callbacks after_create :reset_project_activity diff --git a/app/models/push_event.rb b/app/models/push_event.rb index 23ffb0d4ea8..708513c7861 100644 --- a/app/models/push_event.rb +++ b/app/models/push_event.rb @@ -30,6 +30,44 @@ class PushEvent < Event delegate :commit_count, to: :push_event_payload alias_method :commits_count, :commit_count + # Returns events of pushes that either pushed to an existing ref or created a + # new one. + def self.created_or_pushed + actions = [ + PushEventPayload.actions[:pushed], + PushEventPayload.actions[:created] + ] + + joins(:push_event_payload) + .where(push_event_payloads: { action: actions }) + end + + # Returns events of pushes to a branch. + def self.branch_events + ref_type = PushEventPayload.ref_types[:branch] + + joins(:push_event_payload) + .where(push_event_payloads: { ref_type: ref_type }) + end + + # Returns PushEvent instances for which no merge requests have been created. + def self.without_existing_merge_requests + existing_mrs = MergeRequest.except(:order) + .select(1) + .where('merge_requests.source_project_id = events.project_id') + .where('merge_requests.source_branch = push_event_payloads.ref') + + # For reasons unknown the use of #eager_load will result in the + # "push_event_payload" association not being set. Because of this we're + # using "joins" here, which does mean an additional query needs to be + # executed in order to retrieve the "push_event_association" when the + # returned PushEvent is used. + joins(:push_event_payload) + .where('NOT EXISTS (?)', existing_mrs) + .created_or_pushed + .branch_events + end + def self.sti_name PUSHED end diff --git a/app/models/user.rb b/app/models/user.rb index 26b14ade5ca..358b04ac71f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -650,20 +650,13 @@ class User < ActiveRecord::Base @personal_projects_count ||= personal_projects.count end - def recent_push(project_ids = nil) - # Get push events not earlier than 2 hours ago - events = recent_events.code_push.where("created_at > ?", Time.now - 2.hours) - events = events.where(project_id: project_ids) if project_ids + def recent_push(project = nil) + service = Users::LastPushEventService.new(self) - # Use the latest event that has not been pushed or merged recently - events.includes(:project).recent.find do |event| - next unless event.project.repository.branch_exists?(event.branch_name) - - merge_requests = MergeRequest.where("created_at >= ?", event.created_at) - .where(source_project_id: event.project.id, - source_branch: event.branch_name) - - merge_requests.empty? + if project + service.last_event_for_project(project) + else + service.last_event_for_user end end diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 0b7e4f187f7..6328d567a07 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -74,12 +74,19 @@ class EventCreateService # We're using an explicit transaction here so that any errors that may occur # when creating push payload data will result in the event creation being # rolled back as well. - Event.transaction do - event = create_event(project, current_user, Event::PUSHED) + event = Event.transaction do + new_event = create_event(project, current_user, Event::PUSHED) - PushEventPayloadService.new(event, push_data).execute + PushEventPayloadService + .new(new_event, push_data) + .execute + + new_event end + Users::LastPushEventService.new(current_user) + .cache_last_push_event(event) + Users::ActivityService.new(current_user, 'push').execute end diff --git a/app/services/users/last_push_event_service.rb b/app/services/users/last_push_event_service.rb new file mode 100644 index 00000000000..f2bfb60604f --- /dev/null +++ b/app/services/users/last_push_event_service.rb @@ -0,0 +1,83 @@ +module Users + # Service class for caching and retrieving the last push event of a user. + class LastPushEventService + EXPIRATION = 2.hours + + def initialize(user) + @user = user + end + + # Caches the given push event for the current user in the Rails cache. + # + # event - An instance of PushEvent to cache. + def cache_last_push_event(event) + keys = [ + project_cache_key(event.project), + user_cache_key + ] + + if event.project.forked? + keys << project_cache_key(event.project.forked_from_project) + end + + keys.each { |key| set_key(key, event.id) } + end + + # Returns the last PushEvent for the current user. + # + # This method will return nil if no event was found. + def last_event_for_user + find_cached_event(user_cache_key) + end + + # Returns the last PushEvent for the current user and the given project. + # + # project - An instance of Project for which to retrieve the PushEvent. + # + # This method will return nil if no event was found. + def last_event_for_project(project) + find_cached_event(project_cache_key(project)) + end + + def find_cached_event(cache_key) + event_id = get_key(cache_key) + + return unless event_id + + unless (event = find_event_in_database(event_id)) + # We don't want to keep querying the same data over and over when a + # merge request has been created, thus we remove the key if no event + # (meaning an MR was created) is returned. + Rails.cache.delete(cache_key) + end + + event + end + + private + + def find_event_in_database(id) + PushEvent + .without_existing_merge_requests + .find_by(id: id) + end + + def user_cache_key + "last-push-event/#{@user.id}" + end + + def project_cache_key(project) + "last-push-event/#{@user.id}/#{project.id}" + end + + def get_key(key) + Rails.cache.read(key, raw: true) + end + + def set_key(key, value) + # We're using raw values here since this takes up less space and we don't + # store complex objects. + Rails.cache.write(key, value, raw: true, expires_in: EXPIRATION) + end + end +end |