Merge branch 'osw-introduce-merge-request-statistics' into 'master'

Improve closed/merged events queries performance on Projects::MergeRequestsController#show.json

See merge request gitlab-org/gitlab-ce!15642

6 files changed, 58 insertions, 4 deletions

diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb
index 6328d567a07..44dc90b3462 100644
--- a/app/services/event_create_service.rb
+++ b/app/services/event_create_service.rb
@@ -103,6 +103,6 @@ class EventCreateService
       author_id: current_user.id
     )
 
-    Event.create(attributes)
+    Event.create!(attributes)
   end
 end
diff --git a/app/services/merge_request_metrics_service.rb b/app/services/merge_request_metrics_service.rb
new file mode 100644
index 00000000000..9248de14a53
--- /dev/null
+++ b/app/services/merge_request_metrics_service.rb
@@ -0,0 +1,19 @@
+class MergeRequestMetricsService
+  delegate :update!, to: :@merge_request_metrics
+
+  def initialize(merge_request_metrics)
+    @merge_request_metrics = merge_request_metrics
+  end
+
+  def merge(event)
+    update!(merged_by_id: event.author_id, merged_at: event.created_at)
+  end
+
+  def close(event)
+    update!(latest_closed_by_id: event.author_id, latest_closed_at: event.created_at)
+  end
+
+  def reopen
+    update!(latest_closed_by_id: nil, latest_closed_at: nil)
+  end
+end
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 6b32d65a74b..20a2b50d3de 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -24,6 +24,10 @@ module MergeRequests
 
     private
 
+    def merge_request_metrics_service(merge_request)
+      MergeRequestMetricsService.new(merge_request.metrics)
+    end
+
     def create_assignee_note(merge_request)
       SystemNoteService.change_assignee(
         merge_request, merge_request.project, current_user, merge_request.assignee)
diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb
index 40213c99014..f727ec002e7 100644
--- a/app/services/merge_requests/close_service.rb
+++ b/app/services/merge_requests/close_service.rb
@@ -8,7 +8,7 @@ module MergeRequests
       merge_request.allow_broken = true
 
       if merge_request.close
-        event_service.close_mr(merge_request, current_user)
+        create_event(merge_request)
         create_note(merge_request)
         notification_service.close_mr(merge_request, current_user)
         todo_service.close_merge_request(merge_request, current_user)
@@ -19,5 +19,16 @@ module MergeRequests
 
       merge_request
     end
+
+    private
+
+    def create_event(merge_request)
+      # Making sure MergeRequest::Metrics updates are in sync with
+      # Event creation.
+      Event.transaction do
+        close_event = event_service.close_mr(merge_request, current_user)
+        merge_request_metrics_service(merge_request).close(close_event)
+      end
+    end
   end
 end
diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb
index b1d6bac4d4a..c78e78afcd1 100644
--- a/app/services/merge_requests/post_merge_service.rb
+++ b/app/services/merge_requests/post_merge_service.rb
@@ -9,7 +9,7 @@ module MergeRequests
       close_issues(merge_request)
       todo_service.merge_merge_request(merge_request, current_user)
       merge_request.mark_as_merged
-      create_merge_event(merge_request, current_user)
+      create_event(merge_request)
       create_note(merge_request)
       notification_service.merge_mr(merge_request, current_user)
       execute_hooks(merge_request, 'merge')
@@ -34,5 +34,14 @@ module MergeRequests
     def create_merge_event(merge_request, current_user)
       EventCreateService.new.merge_mr(merge_request, current_user)
     end
+
+    def create_event(merge_request)
+      # Making sure MergeRequest::Metrics updates are in sync with
+      # Event creation.
+      Event.transaction do
+        merge_event = create_merge_event(merge_request, current_user)
+        merge_request_metrics_service(merge_request).merge(merge_event)
+      end
+    end
   end
 end
diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb
index c599a90f9fe..120677a7149 100644
--- a/app/services/merge_requests/reopen_service.rb
+++ b/app/services/merge_requests/reopen_service.rb
@@ -4,7 +4,7 @@ module MergeRequests
       return merge_request unless can?(current_user, :update_merge_request, merge_request)
 
       if merge_request.reopen
-        event_service.reopen_mr(merge_request, current_user)
+        create_event(merge_request)
         create_note(merge_request, 'reopened')
         notification_service.reopen_mr(merge_request, current_user)
         execute_hooks(merge_request, 'reopen')
@@ -16,5 +16,16 @@ module MergeRequests
 
       merge_request
     end
+
+    private
+
+    def create_event(merge_request)
+      # Making sure MergeRequest::Metrics updates are in sync with
+      # Event creation.
+      Event.transaction do
+        event_service.reopen_mr(merge_request, current_user)
+        merge_request_metrics_service(merge_request).reopen
+      end
+    end
   end
 end