diff options
author | James Lopez <james@jameslopez.es> | 2016-12-01 12:44:35 +0100 |
---|---|---|
committer | James Lopez <james@jameslopez.es> | 2017-01-17 11:32:55 +0100 |
commit | 69ecd951a9e0acbc31f0a4b9b02e8a1981ceff1e (patch) | |
tree | 8840bd95a1c558b6672a1375460f1b3e3f2da610 | |
parent | b8056669849729cab5700466a7fae6dc6b2743b2 (diff) | |
download | gitlab-ce-69ecd951a9e0acbc31f0a4b9b02e8a1981ceff1e.tar.gz |
refactor fetcher and fixed specs
-rw-r--r-- | app/controllers/projects/cycle_analytics/events_controller.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/cycle_analytics/base_event.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/cycle_analytics/events_query.rb | 32 | ||||
-rw-r--r-- | lib/gitlab/cycle_analytics/metrics_fetcher.rb | 15 | ||||
-rw-r--r-- | lib/gitlab/database/median.rb | 5 | ||||
-rw-r--r-- | spec/models/cycle_analytics/code_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/cycle_analytics/issue_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/cycle_analytics/plan_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/cycle_analytics/production_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/cycle_analytics/review_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/cycle_analytics/staging_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/cycle_analytics/test_spec.rb | 10 | ||||
-rw-r--r-- | spec/support/cycle_analytics_helpers/test_generation.rb | 18 |
13 files changed, 49 insertions, 67 deletions
diff --git a/app/controllers/projects/cycle_analytics/events_controller.rb b/app/controllers/projects/cycle_analytics/events_controller.rb index d4969c66467..b69d46f2c41 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 eac807af037..2ce9c34d8e8 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 e2b79384c9b..00000000000 --- 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 bd68a0980ca..0542fbfb38d 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 1444d25ebc7..08607c27c09 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 4838b57e353..70f985afefb 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 ce6e99bbec9..e4b6a8f4518 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 bd5a6a77b7a..dc5b04852d6 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 653e203b491..5e99188f318 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 219cd4c0212..45baa5f7006 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 8dffb6b8fe1..77625aad580 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 ac1304beca8..27a117d2d76 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 dc866b11429..35b40d73191 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 |