refactor fetcher and fixed specs

app/controllers/projects/cycle_analytics/events_controller.rb

@@ -48,7 +48,7 @@ module Projects
       end
     
       def cycle_analytics
-        @cycle_analytics ||= ::CycleAnalytics.new(project, options: options(events_params))
+        @cycle_analytics ||= ::CycleAnalytics.new(project, options(events_params))
       end
     
       def events_params

lib/gitlab/cycle_analytics/base_event.rb

@@ -7,7 +7,7 @@ module Gitlab
       attr_reader :start_time_attrs, :end_time_attrs, :projections, :query
       def initialize(fetcher:, options:)
-        @query = EventsQuery.new(fetcher: fetcher)
+        @fetcher = fetcher
         @project = fetcher.project
         @options = options
       end
@@ -39,7 +39,7 @@ module Gitlab
       end
       def event_result
-        @event_result ||= @query.execute(self).to_a
+        @event_result ||= @fetcher.events(self).to_a
       end
       def serialize(_event)

lib/gitlab/cycle_analytics/events_query.rb

-module Gitlab
-  module CycleAnalytics
-    class EventsQuery
-      def initialize(fetcher:)
-        @fetcher = fetcher
-      end
-
-      def execute(stage_class)
-        @stage_class = stage_class
-
-        ActiveRecord::Base.connection.exec_query(query.to_sql)
-      end
-
-      private
-
-      def query
-        base_query = @fetcher.base_query_for(@stage_class.stage)
-        diff_fn = @fetcher.subtract_datetimes_diff(base_query, @stage_class.start_time_attrs, @stage_class.end_time_attrs)
-
-        @stage_class.custom_query(base_query)
-
-        base_query.project(extract_epoch(diff_fn).as('total_time'), *@stage_class.projections).order(@stage_class.order.desc)
-      end
-
-      def extract_epoch(arel_attribute)
-        return arel_attribute unless Gitlab::Database.postgresql?
-
-        Arel.sql(%Q{EXTRACT(EPOCH FROM (#{arel_attribute.to_sql}))})
-      end
-    end
-  end
-end

lib/gitlab/cycle_analytics/metrics_fetcher.rb

@@ -29,6 +29,21 @@ module Gitlab
         median_datetime(cte_table, interval_query, name)
       end
+      def events(stage_class)
+        ActiveRecord::Base.connection.exec_query(events_query(stage_class).to_sql)
+      end
+
+      private
+
+      def events_query(stage_class)
+        base_query = base_query_for(stage_class.stage)
+        diff_fn = subtract_datetimes_diff(base_query, stage_class.start_time_attrs, stage_class.end_time_attrs)
+
+        stage_class.custom_query(base_query)
+
+        base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *stage_class.projections).order(stage_class.order.desc)
+      end
+
       # Join table with a row for every <issue,merge_request> pair (where the merge request
       # closes the given issue) with issue and merge request metrics included. The metrics
       # are loaded with an inner join, so issues / merge requests without metrics are

lib/gitlab/database/median.rb

@@ -103,6 +103,11 @@ module Gitlab
         Arel.sql(%Q{EXTRACT(EPOCH FROM "#{arel_attribute.relation.name}"."#{arel_attribute.name}")})
       end
+      def extract_diff_epoch(diff)
+        return diff unless Gitlab::Database.postgresql?
+
+        Arel.sql(%Q{EXTRACT(EPOCH FROM (#{diff.to_sql}))})
+      end
       # Need to cast '0' to an INTERVAL before we can check if the interval is positive
       def zero_interval
         Arel::Nodes::NamedFunction.new("CAST", [Arel.sql("'0' AS INTERVAL")])

spec/models/cycle_analytics/code_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#code', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalyticsTest.new(project, options: { from: from_date }) }
+  subject { CycleAnalytics.new(project, from: from_date) }
   context 'with deployment' do
     generate_cycle_analytics_spec(
@@ -37,7 +37,7 @@ describe 'CycleAnalytics#code', feature: true do
           deploy_master
         end
-        expect(subject.code).to be_nil
+        expect(subject[:code].median).to be_nil
       end
     end
   end
@@ -69,7 +69,7 @@ describe 'CycleAnalytics#code', feature: true do
           merge_merge_requests_closing_issue(issue)
         end
-        expect(subject.code).to be_nil
+        expect(subject[:code].median).to be_nil
       end
     end
   end

spec/models/cycle_analytics/issue_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#issue', models: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalyticsTest.new(project, options: { from: from_date }) }
+  subject { CycleAnalytics.new(project, from: from_date) }
   generate_cycle_analytics_spec(
     phase: :issue,
@@ -42,7 +42,7 @@ describe 'CycleAnalytics#issue', models: true do
         merge_merge_requests_closing_issue(issue)
       end
-      expect(subject.issue).to be_nil
+      expect(subject[:issue].median).to be_nil
     end
   end
 end

spec/models/cycle_analytics/plan_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#plan', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalyticsTest.new(project, options: { from: from_date }) }
+  subject { CycleAnalytics.new(project, from: from_date) }
   generate_cycle_analytics_spec(
     phase: :plan,
@@ -44,7 +44,7 @@ describe 'CycleAnalytics#plan', feature: true do
       create_merge_request_closing_issue(issue, source_branch: branch_name)
       merge_merge_requests_closing_issue(issue)
-      expect(subject.issue).to be_nil
+      expect(subject[:issue].median).to be_nil
     end
   end
 end

spec/models/cycle_analytics/production_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#production', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalyticsTest.new(project, options: { from: from_date }) }
+  subject { CycleAnalytics.new(project, from: from_date) }
   generate_cycle_analytics_spec(
     phase: :production,
@@ -35,7 +35,7 @@ describe 'CycleAnalytics#production', feature: true do
         deploy_master
       end
-      expect(subject.production).to be_nil
+      expect(subject[:production].median).to be_nil
     end
   end
@@ -48,7 +48,7 @@ describe 'CycleAnalytics#production', feature: true do
         deploy_master(environment: 'staging')
       end
-      expect(subject.production).to be_nil
+      expect(subject[:production].median).to be_nil
     end
   end
 end

spec/models/cycle_analytics/review_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#review', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalyticsTest.new(project, options: { from: from_date }) }
+  subject { CycleAnalytics.new(project, from: from_date) }
   generate_cycle_analytics_spec(
     phase: :review,
@@ -27,7 +27,7 @@ describe 'CycleAnalytics#review', feature: true do
         MergeRequests::MergeService.new(project, user).execute(create(:merge_request))
       end
-      expect(subject.review).to be_nil
+      expect(subject[:review].median).to be_nil
     end
   end
 end

spec/models/cycle_analytics/staging_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#staging', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalyticsTest.new(project, options: { from: from_date }) }
+  subject { CycleAnalytics.new(project, from: from_date) }
   generate_cycle_analytics_spec(
     phase: :staging,
@@ -45,7 +45,7 @@ describe 'CycleAnalytics#staging', feature: true do
         deploy_master
       end
-      expect(subject.staging).to be_nil
+      expect(subject[:staging].median).to be_nil
     end
   end
@@ -58,7 +58,7 @@ describe 'CycleAnalytics#staging', feature: true do
         deploy_master(environment: 'staging')
       end
-      expect(subject.staging).to be_nil
+      expect(subject[:staging].median).to be_nil
     end
   end
 end

spec/models/cycle_analytics/test_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#test', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalyticsTest.new(project, options: { from: from_date }) }
+  subject { CycleAnalytics.new(project, from: from_date) }
   generate_cycle_analytics_spec(
     phase: :test,
@@ -35,7 +35,7 @@ describe 'CycleAnalytics#test', feature: true do
         merge_merge_requests_closing_issue(issue)
       end
-      expect(subject.test).to be_nil
+      expect(subject[:test].median).to be_nil
     end
   end
@@ -48,7 +48,7 @@ describe 'CycleAnalytics#test', feature: true do
         pipeline.succeed!
       end
-      expect(subject.test).to be_nil
+      expect(subject[:test].median).to be_nil
     end
   end
@@ -65,7 +65,7 @@ describe 'CycleAnalytics#test', feature: true do
         merge_merge_requests_closing_issue(issue)
       end
-      expect(subject.test).to be_nil
+      expect(subject[:test].median).to be_nil
     end
   end
@@ -82,7 +82,7 @@ describe 'CycleAnalytics#test', feature: true do
         merge_merge_requests_closing_issue(issue)
       end
-      expect(subject.test).to be_nil
+      expect(subject[:test].median).to be_nil
     end
   end
 end

spec/support/cycle_analytics_helpers/test_generation.rb

-class CycleAnalyticsTest < CycleAnalytics
-  def method_missing(method_sym, *arguments, &block)
-    classify_stage(method_sym).new(project: @project, options: @options, stage: method_sym).median
-  end
-end
-
 # rubocop:disable Metrics/AbcSize
 # Note: The ABC size is large here because we have a method generating test cases with
@@ -56,7 +50,7 @@ module CycleAnalyticsHelpers
               end
               median_time_difference = time_differences.sort[2]
-              expect(subject.public_send(phase)).to be_within(5).of(median_time_difference)
+              expect(subject[phase].median).to be_within(5).of(median_time_difference)
             end
             context "when the data belongs to another project" do
@@ -88,7 +82,7 @@ module CycleAnalyticsHelpers
                 # Turn off the stub before checking assertions
                 allow(self).to receive(:project).and_call_original
-                expect(subject.public_send(phase)).to be_nil
+                expect(subject[phase].median).to be_nil
               end
             end
@@ -111,7 +105,7 @@ module CycleAnalyticsHelpers
                 Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn
-                expect(subject.public_send(phase)).to be_nil
+                expect(subject[phase].median).to be_nil
               end
             end
           end
@@ -131,7 +125,7 @@ module CycleAnalyticsHelpers
                 Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn
               end
-              expect(subject.public_send(phase)).to be_nil
+              expect(subject[phase].median).to be_nil
             end
           end
         end
@@ -150,7 +144,7 @@ module CycleAnalyticsHelpers
                 post_fn[self, data] if post_fn
               end
-              expect(subject.public_send(phase)).to be_nil
+              expect(subject[phase].median).to be_nil
             end
           end
         end
@@ -158,7 +152,7 @@ module CycleAnalyticsHelpers
       context "when none of the start / end conditions are matched" do
         it "returns nil" do
-          expect(subject.public_send(phase)).to be_nil
+          expect(subject[phase].median).to be_nil
         end
       end
     end