Create Ci::DailyBuildGroupReportResult for daily code coverage

This basically moves away from the EAV model which was
Ci::DailyReportResult. Process is still the same, but now
we're using JSONB column to store all the metrics.
For now, it's just coverage.

app/models/ci/daily_report_result.rb → app/models/ci/daily_build_group_report_result.rb

 # frozen_string_literal: true
 
 module Ci
-  class DailyReportResult < ApplicationRecord
+  class DailyBuildGroupReportResult < ApplicationRecord
     extend Gitlab::Ci::Model
 
     belongs_to :last_pipeline, class_name: 'Ci::Pipeline', foreign_key: :last_pipeline_id
     belongs_to :project
 
-    # TODO: Refactor this out when BuildReportResult is implemented.
-    # They both need to share the same enum values for param.
-    REPORT_PARAMS = {
-      coverage: 0
-    }.freeze
-
-    enum param_type: REPORT_PARAMS
-
     def self.upsert_reports(data)
-      upsert_all(data, unique_by: :index_daily_report_results_unique_columns) if data.any?
+      upsert_all(data, unique_by: :index_daily_build_group_report_results_unique_columns) if data.any?
     end
   end
 end

app/models/ci/pipeline.rb

@@ -82,7 +82,7 @@ module Ci
 
     has_one :pipeline_config, class_name: 'Ci::PipelineConfig', inverse_of: :pipeline
 
-    has_many :daily_report_results, class_name: 'Ci::DailyReportResult', foreign_key: :last_pipeline_id
+    has_many :daily_build_group_report_results, class_name: 'Ci::DailyBuildGroupReportResult', foreign_key: :last_pipeline_id
 
     accepts_nested_attributes_for :variables, reject_if: :persisted?
 
@@ -194,7 +194,7 @@ module Ci
         # We wait a little bit to ensure that all BuildFinishedWorkers finish first
         # because this is where some metrics like code coverage is parsed and stored
         # in CI build records which the daily build metrics worker relies on.
-        pipeline.run_after_commit { Ci::DailyReportResultsWorker.perform_in(10.minutes, pipeline.id) }
+        pipeline.run_after_commit { Ci::DailyBuildGroupReportResultsWorker.perform_in(10.minutes, pipeline.id) }
       end
 
       after_transition do |pipeline, transition|

app/models/project.rb

@@ -322,7 +322,7 @@ class Project < ApplicationRecord
   has_many :import_failures, inverse_of: :project
   has_many :jira_imports, -> { order 'jira_imports.created_at' }, class_name: 'JiraImportState', inverse_of: :project
 
-  has_many :daily_report_results, class_name: 'Ci::DailyReportResult'
+  has_many :daily_build_group_report_results, class_name: 'Ci::DailyBuildGroupReportResult'
 
   has_many :repository_storage_moves, class_name: 'ProjectRepositoryStorageMove'
 

app/services/ci/daily_report_result_service.rb → app/services/ci/daily_build_group_report_result_service.rb

 # frozen_string_literal: true
 
 module Ci
-  class DailyReportResultService
+  class DailyBuildGroupReportResultService
     def execute(pipeline)
       return unless Feature.enabled?(:ci_daily_code_coverage, pipeline.project, default_enabled: true)
 
-      DailyReportResult.upsert_reports(coverage_reports(pipeline))
+      DailyBuildGroupReportResult.upsert_reports(coverage_reports(pipeline))
     end
 
     private
@@ -14,15 +14,14 @@ module Ci
       base_attrs = {
         project_id: pipeline.project_id,
         ref_path: pipeline.source_ref_path,
-        param_type: DailyReportResult.param_types[:coverage],
         date: pipeline.created_at.to_date,
         last_pipeline_id: pipeline.id
       }
 
       aggregate(pipeline.builds.with_coverage).map do |group_name, group|
         base_attrs.merge(
-          title: group_name,
-          value: average_coverage(group)
+          group_name: group_name,
+          data: { coverage: average_coverage(group) }
         )
       end
     end

app/workers/all_queues.yml

@@ -682,7 +682,7 @@
   :resource_boundary: :unknown
   :weight: 1
   :idempotent: 
-- :name: pipeline_background:ci_daily_report_results
+- :name: pipeline_background:ci_daily_build_group_report_results
   :feature_category: :continuous_integration
   :has_external_dependencies: 
   :urgency: :low

app/workers/ci/daily_report_results_worker.rb → app/workers/ci/daily_build_group_report_results_worker.rb

 # frozen_string_literal: true
 
 module Ci
-  class DailyReportResultsWorker
+  class DailyBuildGroupReportResultsWorker
     include ApplicationWorker
     include PipelineBackgroundQueue
 
@@ -9,7 +9,7 @@ module Ci
 
     def perform(pipeline_id)
       Ci::Pipeline.find_by_id(pipeline_id).try do |pipeline|
-        Ci::DailyReportResultService.new.execute(pipeline)
+        Ci::DailyBuildGroupReportResultService.new.execute(pipeline)
       end
     end
   end

db/migrate/20200421111005_create_daily_build_group_report_results.rb

+# frozen_string_literal: true
+
+class CreateDailyBuildGroupReportResults < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    create_table :ci_daily_build_group_report_results do |t|
+      t.date :date, null: false
+      t.bigint :project_id, null: false
+      t.bigint :last_pipeline_id, null: false
+      t.text :ref_path, null: false # rubocop:disable Migration/AddLimitToTextColumns
+      t.text :group_name, null: false # rubocop:disable Migration/AddLimitToTextColumns
+      t.jsonb :data, null: false
+
+      t.index :last_pipeline_id
+      t.index [:project_id, :ref_path, :date, :group_name], name: 'index_daily_build_group_report_results_unique_columns', unique: true
+      t.foreign_key :projects, on_delete: :cascade
+      t.foreign_key :ci_pipelines, column: :last_pipeline_id, on_delete: :cascade
+    end
+  end
+end

db/structure.sql

@@ -1025,6 +1025,25 @@ CREATE SEQUENCE public.ci_builds_runner_session_id_seq
 
 ALTER SEQUENCE public.ci_builds_runner_session_id_seq OWNED BY public.ci_builds_runner_session.id;
 
+CREATE TABLE public.ci_daily_build_group_report_results (
+    id bigint NOT NULL,
+    date date NOT NULL,
+    project_id bigint NOT NULL,
+    last_pipeline_id bigint NOT NULL,
+    ref_path text NOT NULL,
+    group_name text NOT NULL,
+    data jsonb NOT NULL
+);
+
+CREATE SEQUENCE public.ci_daily_build_group_report_results_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.ci_daily_build_group_report_results_id_seq OWNED BY public.ci_daily_build_group_report_results.id;
+
 CREATE TABLE public.ci_daily_report_results (
     id bigint NOT NULL,
     date date NOT NULL,
@@ -7285,6 +7304,8 @@ ALTER TABLE ONLY public.ci_builds_metadata ALTER COLUMN id SET DEFAULT nextval('
 
 ALTER TABLE ONLY public.ci_builds_runner_session ALTER COLUMN id SET DEFAULT nextval('public.ci_builds_runner_session_id_seq'::regclass);
 
+ALTER TABLE ONLY public.ci_daily_build_group_report_results ALTER COLUMN id SET DEFAULT nextval('public.ci_daily_build_group_report_results_id_seq'::regclass);
+
 ALTER TABLE ONLY public.ci_daily_report_results ALTER COLUMN id SET DEFAULT nextval('public.ci_daily_report_results_id_seq'::regclass);
 
 ALTER TABLE ONLY public.ci_group_variables ALTER COLUMN id SET DEFAULT nextval('public.ci_group_variables_id_seq'::regclass);
@@ -7954,6 +7975,9 @@ ALTER TABLE ONLY public.ci_builds
 ALTER TABLE ONLY public.ci_builds_runner_session
     ADD CONSTRAINT ci_builds_runner_session_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.ci_daily_build_group_report_results
+    ADD CONSTRAINT ci_daily_build_group_report_results_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY public.ci_daily_report_results
     ADD CONSTRAINT ci_daily_report_results_pkey PRIMARY KEY (id);
 
@@ -9148,6 +9172,8 @@ CREATE INDEX index_ci_builds_project_id_and_status_for_live_jobs_partial2 ON pub
 
 CREATE UNIQUE INDEX index_ci_builds_runner_session_on_build_id ON public.ci_builds_runner_session USING btree (build_id);
 
+CREATE INDEX index_ci_daily_build_group_report_results_on_last_pipeline_id ON public.ci_daily_build_group_report_results USING btree (last_pipeline_id);
+
 CREATE INDEX index_ci_daily_report_results_on_last_pipeline_id ON public.ci_daily_report_results USING btree (last_pipeline_id);
 
 CREATE UNIQUE INDEX index_ci_group_variables_on_group_id_and_key ON public.ci_group_variables USING btree (group_id, key);
@@ -9356,6 +9382,8 @@ CREATE UNIQUE INDEX index_container_repositories_on_project_id_and_name ON publi
 
 CREATE INDEX index_container_repository_on_name_trigram ON public.container_repositories USING gin (name public.gin_trgm_ops);
 
+CREATE UNIQUE INDEX index_daily_build_group_report_results_unique_columns ON public.ci_daily_build_group_report_results USING btree (project_id, ref_path, date, group_name);
+
 CREATE UNIQUE INDEX index_daily_report_results_unique_columns ON public.ci_daily_report_results USING btree (project_id, ref_path, param_type, date, title);
 
 CREATE INDEX index_dependency_proxy_blobs_on_group_id_and_file_name ON public.dependency_proxy_blobs USING btree (group_id, file_name);
@@ -11469,6 +11497,9 @@ ALTER TABLE ONLY public.events
 ALTER TABLE ONLY public.ip_restrictions
     ADD CONSTRAINT fk_rails_04a93778d5 FOREIGN KEY (group_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.ci_daily_build_group_report_results
+    ADD CONSTRAINT fk_rails_0667f7608c FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.ci_subscriptions_projects
     ADD CONSTRAINT fk_rails_0818751483 FOREIGN KEY (downstream_project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
@@ -12414,6 +12445,9 @@ ALTER TABLE ONLY public.ci_daily_report_results
 ALTER TABLE ONLY public.cluster_providers_aws
     ADD CONSTRAINT fk_rails_ed1fdfaeb2 FOREIGN KEY (created_by_user_id) REFERENCES public.users(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY public.ci_daily_build_group_report_results
+    ADD CONSTRAINT fk_rails_ee072d13b3 FOREIGN KEY (last_pipeline_id) REFERENCES public.ci_pipelines(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.label_priorities
     ADD CONSTRAINT fk_rails_ef916d14fa FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
@@ -13656,6 +13690,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200420172927
 20200420201933
 20200421092907
+20200421111005
 20200421233150
 20200422091541
 20200422213749

spec/factories/ci/daily_report_results.rb → spec/factories/ci/daily_build_group_report_results.rb

 # frozen_string_literal: true
 
 FactoryBot.define do
-  factory :ci_daily_report_result, class: 'Ci::DailyReportResult' do
+  factory :ci_daily_build_group_report_result, class: 'Ci::DailyBuildGroupReportResult' do
     ref_path { Gitlab::Git::BRANCH_REF_PREFIX + 'master' }
     date { Time.zone.now.to_date }
     project
     last_pipeline factory: :ci_pipeline
-    param_type { Ci::DailyReportResult.param_types[:coverage] }
-    title { 'rspec' }
-    value { 77.0 }
+    group_name { 'rspec' }
+    data do
+      { coverage: 77.0 }
+    end
   end
 end

spec/lib/gitlab/import_export/all_models.yml

@@ -217,7 +217,7 @@ ci_pipelines:
 - vulnerability_findings
 - pipeline_config
 - security_scans
-- daily_report_results
+- daily_build_group_report_results
 pipeline_variables:
 - pipeline
 stages:
@@ -484,7 +484,7 @@ project:
 - status_page_setting
 - requirements
 - export_jobs
-- daily_report_results
+- daily_build_group_report_results
 - jira_imports
 - compliance_framework_setting
 - metrics_users_starred_dashboards

spec/models/ci/daily_report_result_spec.rb → spec/models/ci/daily_build_group_report_result_spec.rb

@@ -2,14 +2,14 @@
 
 require 'spec_helper'
 
-describe Ci::DailyReportResult do
+describe Ci::DailyBuildGroupReportResult do
   describe '.upsert_reports' do
     let!(:rspec_coverage) do
       create(
-        :ci_daily_report_result,
-        title: 'rspec',
+        :ci_daily_build_group_report_result,
+        group_name: 'rspec',
         date: '2020-03-09',
-        value: 71.2
+        data: { coverage: 71.2 }
       )
     end
     let!(:new_pipeline) { create(:ci_pipeline) }
@@ -19,20 +19,18 @@ describe Ci::DailyReportResult do
         {
           project_id: rspec_coverage.project_id,
           ref_path: rspec_coverage.ref_path,
-          param_type: described_class.param_types[rspec_coverage.param_type],
           last_pipeline_id: new_pipeline.id,
           date: rspec_coverage.date,
-          title: 'rspec',
-          value: 81.0
+          group_name: 'rspec',
+          data: { 'coverage' => 81.0 }
         },
         {
           project_id: rspec_coverage.project_id,
           ref_path: rspec_coverage.ref_path,
-          param_type: described_class.param_types[rspec_coverage.param_type],
           last_pipeline_id: new_pipeline.id,
           date: rspec_coverage.date,
-          title: 'karma',
-          value: 87.0
+          group_name: 'karma',
+          data: { 'coverage' => 87.0 }
         }
       ])
 
@@ -40,16 +38,15 @@ describe Ci::DailyReportResult do
 
       expect(rspec_coverage).to have_attributes(
         last_pipeline_id: new_pipeline.id,
-        value: 81.0
+        data: { 'coverage' => 81.0 }
       )
 
-      expect(described_class.find_by_title('karma')).to have_attributes(
+      expect(described_class.find_by_group_name('karma')).to have_attributes(
         project_id: rspec_coverage.project_id,
         ref_path: rspec_coverage.ref_path,
-        param_type: rspec_coverage.param_type,
         last_pipeline_id: new_pipeline.id,
         date: rspec_coverage.date,
-        value: 87.0
+        data: { 'coverage' => 87.0 }
       )
     end
 

spec/models/ci/pipeline_spec.rb

@@ -1163,8 +1163,8 @@ describe Ci::Pipeline, :mailer do
         context "from #{status}" do
           let(:from_status) { status }
 
-          it 'schedules pipeline success worker' do
-            expect(Ci::DailyReportResultsWorker).to receive(:perform_in).with(10.minutes, pipeline.id)
+          it 'schedules daily build group report results worker' do
+            expect(Ci::DailyBuildGroupReportResultsWorker).to receive(:perform_in).with(10.minutes, pipeline.id)
 
             pipeline.succeed
           end

spec/services/ci/daily_report_result_service_spec.rb → spec/services/ci/daily_build_group_report_result_service_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Ci::DailyReportResultService, '#execute' do
+describe Ci::DailyBuildGroupReportResultService, '#execute' do
   let!(:pipeline) { create(:ci_pipeline, created_at: '2020-02-06 00:01:10') }
   let!(:rspec_job) { create(:ci_build, pipeline: pipeline, name: '3/3 rspec', coverage: 80) }
   let!(:karma_job) { create(:ci_build, pipeline: pipeline, name: '2/2 karma', coverage: 90) }
@@ -11,31 +11,29 @@ describe Ci::DailyReportResultService, '#execute' do
   it 'creates daily code coverage record for each job in the pipeline that has coverage value' do
     described_class.new.execute(pipeline)
 
-    Ci::DailyReportResult.find_by(title: 'rspec').tap do |coverage|
+    Ci::DailyBuildGroupReportResult.find_by(group_name: 'rspec').tap do |coverage|
       expect(coverage).to have_attributes(
         project_id: pipeline.project.id,
         last_pipeline_id: pipeline.id,
         ref_path: pipeline.source_ref_path,
-        param_type: 'coverage',
-        title: rspec_job.group_name,
-        value: rspec_job.coverage,
+        group_name: rspec_job.group_name,
+        data: { 'coverage' => rspec_job.coverage },
         date: pipeline.created_at.to_date
       )
     end
 
-    Ci::DailyReportResult.find_by(title: 'karma').tap do |coverage|
+    Ci::DailyBuildGroupReportResult.find_by(group_name: 'karma').tap do |coverage|
       expect(coverage).to have_attributes(
         project_id: pipeline.project.id,
         last_pipeline_id: pipeline.id,
         ref_path: pipeline.source_ref_path,
-        param_type: 'coverage',
-        title: karma_job.group_name,
-        value: karma_job.coverage,
+        group_name: karma_job.group_name,
+        data: { 'coverage' => karma_job.coverage },
         date: pipeline.created_at.to_date
       )
     end
 
-    expect(Ci::DailyReportResult.find_by(title: 'extra')).to be_nil
+    expect(Ci::DailyBuildGroupReportResult.find_by(group_name: 'extra')).to be_nil
   end
 
   context 'when there are multiple builds with the same group name that report coverage' do
@@ -45,14 +43,13 @@ describe Ci::DailyReportResultService, '#execute' do
     it 'creates daily code coverage record with the average as the value' do
       described_class.new.execute(pipeline)
 
-      Ci::DailyReportResult.find_by(title: 'test').tap do |coverage|
+      Ci::DailyBuildGroupReportResult.find_by(group_name: 'test').tap do |coverage|
         expect(coverage).to have_attributes(
           project_id: pipeline.project.id,
           last_pipeline_id: pipeline.id,
           ref_path: pipeline.source_ref_path,
-          param_type: 'coverage',
-          title: test_job_2.group_name,
-          value: 75,
+          group_name: test_job_2.group_name,
+          data: { 'coverage' => 75.0 },
           date: pipeline.created_at.to_date
         )
       end
@@ -77,8 +74,8 @@ describe Ci::DailyReportResultService, '#execute' do
     end
 
     it "updates the existing record's coverage value and last_pipeline_id" do
-      rspec_coverage = Ci::DailyReportResult.find_by(title: 'rspec')
-      karma_coverage = Ci::DailyReportResult.find_by(title: 'karma')
+      rspec_coverage = Ci::DailyBuildGroupReportResult.find_by(group_name: 'rspec')
+      karma_coverage = Ci::DailyBuildGroupReportResult.find_by(group_name: 'karma')
 
       # Bump up the coverage values
       described_class.new.execute(new_pipeline)
@@ -88,12 +85,12 @@ describe Ci::DailyReportResultService, '#execute' do
 
       expect(rspec_coverage).to have_attributes(
         last_pipeline_id: new_pipeline.id,
-        value: new_rspec_job.coverage
+        data: { 'coverage' => new_rspec_job.coverage }
       )
 
       expect(karma_coverage).to have_attributes(
         last_pipeline_id: new_pipeline.id,
-        value: new_karma_job.coverage
+        data: { 'coverage' => new_karma_job.coverage }
       )
     end
   end
@@ -117,8 +114,8 @@ describe Ci::DailyReportResultService, '#execute' do
     end
 
     it 'updates the existing daily code coverage records' do
-      rspec_coverage = Ci::DailyReportResult.find_by(title: 'rspec')
-      karma_coverage = Ci::DailyReportResult.find_by(title: 'karma')
+      rspec_coverage = Ci::DailyBuildGroupReportResult.find_by(group_name: 'rspec')
+      karma_coverage = Ci::DailyBuildGroupReportResult.find_by(group_name: 'karma')
 
       # Run another one but for the older pipeline.
       # This simulates the scenario wherein the success worker
@@ -135,12 +132,12 @@ describe Ci::DailyReportResultService, '#execute' do
 
       expect(rspec_coverage).to have_attributes(
         last_pipeline_id: pipeline.id,
-        value: rspec_job.coverage
+        data: { 'coverage' => rspec_job.coverage }
       )
 
       expect(karma_coverage).to have_attributes(
         last_pipeline_id: pipeline.id,
-        value: karma_job.coverage
+        data: { 'coverage' => karma_job.coverage }
       )
     end
   end

spec/workers/ci/daily_report_results_worker_spec.rb → spec/workers/ci/daily_build_group_report_results_worker_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Ci::DailyReportResultsWorker do
+describe Ci::DailyBuildGroupReportResultsWorker do
   describe '#perform' do
     let!(:pipeline) { create(:ci_pipeline) }
 
@@ -12,7 +12,7 @@ describe Ci::DailyReportResultsWorker do
       let(:pipeline_id) { pipeline.id }
 
       it 'executes service' do
-        expect_any_instance_of(Ci::DailyReportResultService)
+        expect_any_instance_of(Ci::DailyBuildGroupReportResultService)
           .to receive(:execute).with(pipeline)
 
         subject
@@ -23,7 +23,7 @@ describe Ci::DailyReportResultsWorker do
       let(:pipeline_id) { 123 }
 
       it 'does not execute service' do
-        expect_any_instance_of(Ci::DailyReportResultService)
+        expect_any_instance_of(Ci::DailyBuildGroupReportResultService)
           .not_to receive(:execute)
 
         expect { subject }