diff --git a/app/controllers/projects/cycle_analytics/events_controller.rb b/app/controllers/projects/cycle_analytics/events_controller.rb index d4969c66467bd87d2624c02e85310157dcdbbe0e..b69d46f2c41acf0966316243c25865e9cb71c9a7 100644 --- a/app/controllers/projects/cycle_analytics/events_controller.rb +++ b/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 diff --git a/lib/gitlab/cycle_analytics/base_event.rb b/lib/gitlab/cycle_analytics/base_event.rb index eac807af037c1d67751c312643665d54cd3a1902..2ce9c34d8e847a5f7eee6e9c060fca9a8477af59 100644 --- a/lib/gitlab/cycle_analytics/base_event.rb +++ b/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) diff --git a/lib/gitlab/cycle_analytics/events_query.rb b/lib/gitlab/cycle_analytics/events_query.rb deleted file mode 100644 index e2b79384c9bedd35cd00f39cb4cdc9c9d539584a..0000000000000000000000000000000000000000 --- a/lib/gitlab/cycle_analytics/events_query.rb +++ /dev/null @@ -1,32 +0,0 @@ -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 diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb index bd68a0980cacbba177ae0f1a674db9fc1bd2c7fc..0542fbfb38d2388a3b41073c923530d90c209eef 100644 --- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb +++ b/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 diff --git a/lib/gitlab/database/median.rb b/lib/gitlab/database/median.rb index 1444d25ebc7563540ded9cd95846158ad369a349..08607c27c0987470f9131a188faa44257811de6d 100644 --- a/lib/gitlab/database/median.rb +++ b/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")]) diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb index 4838b57e3533370b764a4783ceb151c4e6f240ad..70f985afefb1bba0292186f4c87237a0a1d2890b 100644 --- a/spec/models/cycle_analytics/code_spec.rb +++ b/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 diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb index ce6e99bbec9d9c571df90b534bafd8de59710322..e4b6a8f45180753656cb040af935659436b4e975 100644 --- a/spec/models/cycle_analytics/issue_spec.rb +++ b/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 diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index bd5a6a77b7a3d32d05adc4331535bc850d7aa2cd..dc5b04852d668c8a40ebd625ca03ad53d2683977 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/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 diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb index 653e203b491d07cffe785e4db7431dab9820a68e..5e99188f318e31dd0e179da0c244b377d4ca2a51 100644 --- a/spec/models/cycle_analytics/production_spec.rb +++ b/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 diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb index 219cd4c02122dc8fa1b2c9e7f43cafd48521e7a6..45baa5f700644c70453029050ad6ce70fbad0089 100644 --- a/spec/models/cycle_analytics/review_spec.rb +++ b/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 diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb index 8dffb6b8fe150aeefbd7ade4363161b74e1d0781..77625aad5806495bdf5f046b13a41815ae09314e 100644 --- a/spec/models/cycle_analytics/staging_spec.rb +++ b/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 diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb index ac1304beca8048226d818b319dde8b578545eee0..27a117d2d76ba832eb0d11219ebe2218ba47224e 100644 --- a/spec/models/cycle_analytics/test_spec.rb +++ b/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 diff --git a/spec/support/cycle_analytics_helpers/test_generation.rb b/spec/support/cycle_analytics_helpers/test_generation.rb index dc866b11429d28a86228c23d554d69736ba8fa6e..35b40d731913c87ca3ba3f7971795630a0dd275e 100644 --- a/spec/support/cycle_analytics_helpers/test_generation.rb +++ b/spec/support/cycle_analytics_helpers/test_generation.rb @@ -1,9 +1,3 @@ -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