From d7eadcc0f30d3bd005f9dfb160dd0a460b3a8f56 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Wed, 24 Jul 2019 07:42:45 -0700
Subject: [PATCH] Use a base class for Peek views

Introduce a `DetailedView` base class, which is inherited by
the Gitaly, Redis, and Rugged views. This reduces code duplication.
---
 lib/peek/views/detailed_view.rb    | 49 ++++++++++++++++++++++++++++++
 lib/peek/views/gitaly.rb           | 27 +++-------------
 lib/peek/views/redis_detailed.rb   | 23 ++------------
 lib/peek/views/rugged.rb           | 35 ++++++---------------
 spec/lib/peek/views/rugged_spec.rb |  3 --
 5 files changed, 65 insertions(+), 72 deletions(-)
 create mode 100644 lib/peek/views/detailed_view.rb

diff --git a/lib/peek/views/detailed_view.rb b/lib/peek/views/detailed_view.rb
new file mode 100644
index 00000000000..ebaf46478df
--- /dev/null
+++ b/lib/peek/views/detailed_view.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module Peek
+  module Views
+    class DetailedView < View
+      def results
+        {
+          duration: formatted_duration,
+          calls: calls,
+          details: details
+        }
+      end
+
+      private
+
+      def duration
+        raise NotImplementedError
+      end
+
+      def calls
+        raise NotImplementedError
+      end
+
+      def call_details
+        raise NotImplementedError
+      end
+
+      def format_call_details(call)
+        raise NotImplementedError
+      end
+
+      def details
+        call_details
+          .sort { |a, b| b[:duration] <=> a[:duration] }
+          .map(&method(:format_call_details))
+      end
+
+      def formatted_duration
+        ms = duration * 1000
+
+        if ms >= 1000
+          "%.2fms" % ms
+        else
+          "%.0fms" % ms
+        end
+      end
+    end
+  end
+end
diff --git a/lib/peek/views/gitaly.rb b/lib/peek/views/gitaly.rb
index 30f95a10024..067aaf31fbc 100644
--- a/lib/peek/views/gitaly.rb
+++ b/lib/peek/views/gitaly.rb
@@ -2,7 +2,9 @@
 
 module Peek
   module Views
-    class Gitaly < View
+    class Gitaly < DetailedView
+      private
+
       def duration
         ::Gitlab::GitalyClient.query_time
       end
@@ -11,20 +13,8 @@ module Peek
         ::Gitlab::GitalyClient.get_request_count
       end
 
-      def results
-        {
-          duration: formatted_duration,
-          calls: calls,
-          details: details
-        }
-      end
-
-      private
-
-      def details
+      def call_details
         ::Gitlab::GitalyClient.list_call_details
-          .sort { |a, b| b[:duration] <=> a[:duration] }
-          .map(&method(:format_call_details))
       end
 
       def format_call_details(call)
@@ -34,15 +24,6 @@ module Peek
                    request: pretty_request || {})
       end
 
-      def formatted_duration
-        ms = duration * 1000
-        if ms >= 1000
-          "%.2fms" % ms
-        else
-          "%.0fms" % ms
-        end
-      end
-
       def setup_subscribers
         subscribe 'start_processing.action_controller' do
           ::Gitlab::GitalyClient.query_time = 0
diff --git a/lib/peek/views/redis_detailed.rb b/lib/peek/views/redis_detailed.rb
index 12760c9b75e..c61a1e91282 100644
--- a/lib/peek/views/redis_detailed.rb
+++ b/lib/peek/views/redis_detailed.rb
@@ -35,36 +35,19 @@ end
 
 module Peek
   module Views
-    class RedisDetailed < View
+    class RedisDetailed < DetailedView
       REDACTED_MARKER = "<redacted>"
 
       def key
         'redis'
       end
 
-      def results
-        {
-          calls: calls,
-          duration: formatted_duration,
-          details: details
-        }
-      end
-
       def detail_store
         ::Gitlab::SafeRequestStore['redis_call_details'] ||= []
       end
 
       private
 
-      def formatted_duration
-        ms = duration * 1000
-        if ms >= 1000
-          "%.2fms" % ms
-        else
-          "%.0fms" % ms
-        end
-      end
-
       def duration
         detail_store.map { |entry| entry[:duration] }.sum # rubocop:disable CodeReuse/ActiveRecord
       end
@@ -73,10 +56,8 @@ module Peek
         detail_store.count
       end
 
-      def details
+      def call_details
         detail_store
-          .sort { |a, b| b[:duration] <=> a[:duration] }
-          .map(&method(:format_call_details))
       end
 
       def format_call_details(call)
diff --git a/lib/peek/views/rugged.rb b/lib/peek/views/rugged.rb
index 7b0ab162b15..f0cd520fb8b 100644
--- a/lib/peek/views/rugged.rb
+++ b/lib/peek/views/rugged.rb
@@ -2,7 +2,15 @@
 
 module Peek
   module Views
-    class Rugged < View
+    class Rugged < DetailedView
+      def results
+        return {} unless calls > 0
+
+        super
+      end
+
+      private
+
       def duration
         ::Gitlab::RuggedInstrumentation.query_time
       end
@@ -11,22 +19,8 @@ module Peek
         ::Gitlab::RuggedInstrumentation.query_count
       end
 
-      def results
-        return {} unless calls > 0
-
-        {
-          duration: formatted_duration,
-          calls: calls,
-          details: details
-        }
-      end
-
-      private
-
-      def details
+      def call_details
         ::Gitlab::RuggedInstrumentation.list_call_details
-          .sort { |a, b| b[:duration] <=> a[:duration] }
-          .map(&method(:format_call_details))
       end
 
       def format_call_details(call)
@@ -44,15 +38,6 @@ module Peek
           end
         end
       end
-
-      def formatted_duration
-        ms = duration * 1000
-        if ms >= 1000
-          "%.2fms" % ms
-        else
-          "%.0fms" % ms
-        end
-      end
     end
   end
 end
diff --git a/spec/lib/peek/views/rugged_spec.rb b/spec/lib/peek/views/rugged_spec.rb
index 715b360953c..8bf996fc6bc 100644
--- a/spec/lib/peek/views/rugged_spec.rb
+++ b/spec/lib/peek/views/rugged_spec.rb
@@ -27,9 +27,6 @@ describe Peek::Views::Rugged, :request_store do
                                                      args: [project.repository.raw, 'refs/heads/master'],
                                                      duration: 0.456)
 
-    expect(subject.duration).to be_within(0.00001).of(1.234)
-    expect(subject.calls).to eq(2)
-
     results = subject.results
     expect(results[:calls]).to eq(2)
     expect(results[:duration]).to eq('1234.00ms')
-- 
GitLab