Add BuildReportResult data model

This new model will be used to persit junit counter data in order
to improve performance and remove the feature flag

app/models/ci/build.rb

@@ -39,6 +39,7 @@ module Ci
     has_one :resource, class_name: 'Ci::Resource', inverse_of: :build
     has_many :trace_sections, class_name: 'Ci::BuildTraceSection'
     has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id
+    has_many :report_results, class_name: 'Ci::BuildReportResult', inverse_of: :build
 
     has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy, inverse_of: :job # rubocop:disable Cop/ActiveRecordDependent
     has_many :job_variables, class_name: 'Ci::JobVariable', foreign_key: :job_id

app/models/ci/build_report_result.rb

+# frozen_string_literal: true
+
+module Ci
+  class BuildReportResult < ApplicationRecord
+    extend Gitlab::Ci::Model
+
+    self.primary_key = :build_id
+
+    belongs_to :build, class_name: "Ci::Build", inverse_of: :report_results
+    belongs_to :project, class_name: "Project", inverse_of: :build_report_results
+
+    validates :build, :project, presence: true
+    validates :data, json_schema: { filename: "build_report_result_data" }
+  end
+end

app/models/project.rb

@@ -291,6 +291,7 @@ class Project < ApplicationRecord
   has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :build_trace_section_names, class_name: 'Ci::BuildTraceSectionName'
   has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks
+  has_many :build_report_results, class_name: 'Ci::BuildReportResult', inverse_of: :project
   has_many :job_artifacts, class_name: 'Ci::JobArtifact'
   has_many :runner_projects, class_name: 'Ci::RunnerProject', inverse_of: :project
   has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'

app/validators/json_schema_validator.rb

+# frozen_string_literal: true
+
+require "json-schema"
+
+# JsonSchemaValidator
+#
+# Custom validator for json schema.
+# Create a json schema within the json_schemas directory
+#
+#   class Project < ActiveRecord::Base
+#     validates :data, json_schema: { filename: "file" }
+#   end
+#
+class JsonSchemaValidator < ActiveModel::EachValidator
+  def initialize(options)
+    raise ArgumentError, "Expected 'filename' as an argument" unless options[:filename]
+
+    super(options)
+  end
+
+  def validate_each(record, attribute, value)
+    unless valid_schema?(value)
+      record.errors.add(attribute, "must be a valid json schema")
+    end
+  end
+
+  private
+
+  def valid_schema?(value)
+    JSON::Validator.validate(schema_path, value)
+  end
+
+  def schema_path
+    Rails.root.join('app', 'validators', 'json_schemas', "#{options[:filename]}.json").to_s
+  end
+end

app/validators/json_schemas/build_report_result_data.json

+{
+  "description": "Build report result data",
+  "type": "object",
+  "properties": {
+    "coverage": { "type": "float" },
+    "junit": {
+      "type": "object",
+      "items": { "$ref": "./build_report_result_data_junit.json" }
+    }
+  },
+  "additionalProperties": false
+}

app/validators/json_schemas/build_report_result_data_junit.json

+{
+  "description": "Build report result data junit",
+  "type": "object",
+  "properties": {
+    "name": { "type": "string" },
+    "duration": { "type": "string" },
+    "failed": { "type": "integer" },
+    "errored": { "type": "integer" },
+    "skipped": { "type": "integer" },
+    "success": { "type": "integer" }
+  },
+  "additionalProperties": false
+}

changelogs/unreleased/mo-add-build-report-result.yml

+---
+title: Add build report results data model
+merge_request: 32991
+author:
+type: performance

db/migrate/20200527151413_create_ci_build_report_results_table.rb

+# frozen_string_literal: true
+
+class CreateCiBuildReportResultsTable < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    create_table :ci_build_report_results, id: false do |t|
+      t.bigint :build_id, null: false, index: false, primary_key: true
+      t.bigint :project_id, null: false, index: true
+      t.jsonb :data, null: false, default: {}
+    end
+  end
+end

db/migrate/20200527152116_add_foreign_key_to_build_id_on_build_report_results.rb

+# frozen_string_literal: true
+
+class AddForeignKeyToBuildIdOnBuildReportResults < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_foreign_key :ci_build_report_results, :ci_builds, column: :build_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :ci_build_report_results, column: :build_id
+    end
+  end
+end

db/migrate/20200527152657_add_foreign_key_to_project_id_on_build_report_results.rb

+# frozen_string_literal: true
+
+class AddForeignKeyToProjectIdOnBuildReportResults < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_foreign_key :ci_build_report_results, :projects, column: :project_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :ci_build_report_results, column: :project_id
+    end
+  end
+end

db/structure.sql

@@ -891,6 +891,21 @@ CREATE SEQUENCE public.ci_build_needs_id_seq
 
 ALTER SEQUENCE public.ci_build_needs_id_seq OWNED BY public.ci_build_needs.id;
 
+CREATE TABLE public.ci_build_report_results (
+    build_id bigint NOT NULL,
+    project_id bigint NOT NULL,
+    data jsonb DEFAULT '{}'::jsonb NOT NULL
+);
+
+CREATE SEQUENCE public.ci_build_report_results_build_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.ci_build_report_results_build_id_seq OWNED BY public.ci_build_report_results.build_id;
+
 CREATE TABLE public.ci_build_trace_chunks (
     id bigint NOT NULL,
     build_id integer NOT NULL,
@@ -7383,6 +7398,8 @@ ALTER TABLE ONLY public.chat_teams ALTER COLUMN id SET DEFAULT nextval('public.c
 
 ALTER TABLE ONLY public.ci_build_needs ALTER COLUMN id SET DEFAULT nextval('public.ci_build_needs_id_seq'::regclass);
 
+ALTER TABLE ONLY public.ci_build_report_results ALTER COLUMN build_id SET DEFAULT nextval('public.ci_build_report_results_build_id_seq'::regclass);
+
 ALTER TABLE ONLY public.ci_build_trace_chunks ALTER COLUMN id SET DEFAULT nextval('public.ci_build_trace_chunks_id_seq'::regclass);
 
 ALTER TABLE ONLY public.ci_build_trace_section_names ALTER COLUMN id SET DEFAULT nextval('public.ci_build_trace_section_names_id_seq'::regclass);
@@ -8064,6 +8081,9 @@ ALTER TABLE public.lfs_objects
 ALTER TABLE ONLY public.ci_build_needs
     ADD CONSTRAINT ci_build_needs_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.ci_build_report_results
+    ADD CONSTRAINT ci_build_report_results_pkey PRIMARY KEY (build_id);
+
 ALTER TABLE ONLY public.ci_build_trace_chunks
     ADD CONSTRAINT ci_build_trace_chunks_pkey PRIMARY KEY (id);
 
@@ -9228,6 +9248,8 @@ CREATE UNIQUE INDEX index_chat_teams_on_namespace_id ON public.chat_teams USING
 
 CREATE UNIQUE INDEX index_ci_build_needs_on_build_id_and_name ON public.ci_build_needs USING btree (build_id, name);
 
+CREATE INDEX index_ci_build_report_results_on_project_id ON public.ci_build_report_results USING btree (project_id);
+
 CREATE UNIQUE INDEX index_ci_build_trace_chunks_on_build_id_and_chunk_index ON public.ci_build_trace_chunks USING btree (build_id, chunk_index);
 
 CREATE UNIQUE INDEX index_ci_build_trace_section_names_on_project_id_and_name ON public.ci_build_trace_section_names USING btree (project_id, name);
@@ -11649,6 +11671,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_build_report_results
+    ADD CONSTRAINT fk_rails_056d298d48 FOREIGN KEY (project_id) REFERENCES public.projects(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;
 
@@ -11718,6 +11743,9 @@ ALTER TABLE ONLY public.diff_note_positions
 ALTER TABLE ONLY public.users_security_dashboard_projects
     ADD CONSTRAINT fk_rails_150cd5682c FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.ci_build_report_results
+    ADD CONSTRAINT fk_rails_16cb1ff064 FOREIGN KEY (build_id) REFERENCES public.ci_builds(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.project_deploy_tokens
     ADD CONSTRAINT fk_rails_170e03cbaf FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
@@ -13968,5 +13996,8 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200526164947
 20200527094322
 20200527095401
+20200527151413
+20200527152116
+20200527152657
 \.
 

spec/factories/ci/build_report_results.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :ci_build_report_result, class: 'Ci::BuildReportResult' do
+    build factory: :ci_build
+    project factory: :project
+    data do
+      {
+        junit: {
+          name: "rspec",
+          duration: 0.42,
+          failed: 0,
+          errored: 2,
+          skipped: 0,
+          success: 0
+        }
+      }
+    end
+
+    trait :with_junit_success do
+      data do
+        {
+          junit: {
+            name: "rspec",
+            duration: 0.42,
+            failed: 0,
+            errored: 0,
+            skipped: 0,
+            success: 2
+          }
+        }
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -495,6 +495,7 @@ project:
 - repository_storage_moves
 - freeze_periods
 - webex_teams_service
+- build_report_results
 award_emoji:
 - awardable
 - user

spec/models/ci/build_report_result_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::BuildReportResult do
+  let(:build_report_result) { build(:ci_build_report_result, :with_junit_success) }
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:build) }
+    it { is_expected.to belong_to(:project) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:project) }
+    it { is_expected.to validate_presence_of(:build) }
+
+    context 'when attributes are valid' do
+      it 'returns no errors' do
+        expect(build_report_result).to be_valid
+      end
+    end
+
+    context 'when data is invalid' do
+      it 'returns errors' do
+        build_report_result.data = { invalid: 'data' }
+
+        expect(build_report_result).to be_invalid
+        expect(build_report_result.errors.full_messages).to eq(["Data must be a valid json schema"])
+      end
+    end
+  end
+end

spec/models/ci/build_spec.rb

@@ -24,6 +24,7 @@ describe Ci::Build do
   it { is_expected.to have_many(:needs) }
   it { is_expected.to have_many(:sourced_pipelines) }
   it { is_expected.to have_many(:job_variables) }
+  it { is_expected.to have_many(:report_results) }
 
   it { is_expected.to have_one(:deployment) }
   it { is_expected.to have_one(:runner_session) }

spec/models/project_spec.rb

@@ -79,6 +79,7 @@ describe Project do
     it { is_expected.to have_many(:ci_refs) }
     it { is_expected.to have_many(:builds) }
     it { is_expected.to have_many(:build_trace_section_names)}
+    it { is_expected.to have_many(:build_report_results) }
     it { is_expected.to have_many(:runner_projects) }
     it { is_expected.to have_many(:runners) }
     it { is_expected.to have_many(:variables) }

spec/services/ci/retry_build_service_spec.rb

@@ -49,7 +49,7 @@ describe Ci::RetryBuildService do
        metadata runner_session trace_chunks upstream_pipeline_id
        artifacts_file artifacts_metadata artifacts_size commands
        resource resource_group_id processed security_scans author
-       pipeline_id].freeze
+       pipeline_id report_results].freeze
 
   shared_examples 'build duplication' do
     let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }

spec/validators/json_schema_validator_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe JsonSchemaValidator do
+  describe '#validates_each' do
+    let(:build_report_result) { build(:ci_build_report_result, :with_junit_success) }
+
+    subject { validator.validate(build_report_result) }
+
+    context 'when file_path is set' do
+      let(:validator) { described_class.new(attributes: [:data], filename: "build_report_result_data") }
+
+      context 'when data is valid' do
+        it 'returns no errors' do
+          subject
+
+          expect(build_report_result.errors).to be_empty
+        end
+      end
+
+      context 'when data is invalid' do
+        it 'returns json schema is invalid' do
+          build_report_result.data = { invalid: 'data' }
+
+          validator.validate(build_report_result)
+
+          expect(build_report_result.errors.size).to eq(1)
+          expect(build_report_result.errors.full_messages).to eq(["Data must be a valid json schema"])
+        end
+      end
+    end
+
+    context 'when file_path is not set' do
+      let(:validator) { described_class.new(attributes: [:data]) }
+
+      it 'raises an ArgumentError' do
+        expect { subject }.to raise_error(ArgumentError)
+      end
+    end
+  end
+end