WIP: Add support for CI build annotationsbuild-annotations

5 files changed, 497 insertions, 0 deletions

diff --git a/spec/lib/gitlab/ci/parsers/build_annotation_spec.rb b/spec/lib/gitlab/ci/parsers/build_annotation_spec.rb
new file mode 100644
index 00000000000..1b2c262f096
--- /dev/null
+++ b/spec/lib/gitlab/ci/parsers/build_annotation_spec.rb
@@ -0,0 +1,213 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Parsers::BuildAnnotation do
+  let(:parser) { described_class.new }
+
+  describe '#parse!' do
+    context 'when the annotations artifact is too large' do
+      it 'raises a ParserError' do
+        artifact = double(:artifact, job_id: 1, size: 20.megabytes)
+
+        expect { parser.parse!(artifact) }
+          .to raise_error(described_class::ParserError)
+      end
+    end
+
+    context 'when the annotations artifact is small enough' do
+      it 'parses the JSON and returns the build annotations' do
+        json = JSON.dump([
+          {
+            'severity' => 'info',
+            'summary' => 'summary here',
+            'description' => 'description here',
+            'line_number' => 14,
+            'file_path' => 'README.md'
+          }
+        ])
+
+        artifact = double(:artifact, job_id: 1, size: 4)
+
+        allow(artifact)
+          .to receive(:each_blob)
+          .and_yield(json)
+
+        annotations = parser.parse!(artifact)
+
+        expect(annotations).to eq([
+          {
+            build_id: 1,
+            severity: Ci::BuildAnnotation.severities[:info],
+            summary: 'summary here',
+            description: 'description here',
+            line_number: 14,
+            file_path: 'README.md'
+          }
+        ])
+      end
+    end
+  end
+
+  describe '#parse_json' do
+    context 'when the JSON object contains too many nested objects' do
+      it 'raises a ParserError' do
+        json = JSON.dump([[[[[[{}]]]]]])
+
+        expect { parser.parse_json(json) }
+          .to raise_error(described_class::ParserError)
+      end
+    end
+
+    context 'when the JSON contains too many entries' do
+      it 'raises a ParserError' do
+        json = JSON.dump(Array.new(described_class::MAX_ENTRIES + 1) { {} })
+
+        expect { parser.parse_json(json) }
+          .to raise_error(described_class::ParserError)
+      end
+    end
+
+    context 'when the JSON is valid' do
+      it 'returns the annotations' do
+        json = JSON.dump([{}])
+
+        expect(parser.parse_json(json)).to eq([{}])
+      end
+    end
+  end
+
+  describe '#build_annotation_rows' do
+    context 'when one of the entries is not a JSON object' do
+      it 'raises a ParserError' do
+        artifact = double(:artifact)
+
+        expect { parser.build_annotation_rows(artifact, [10]) }
+          .to raise_error(described_class::ParserError)
+      end
+    end
+
+    context 'when one of the JSON objects is invalid' do
+      it 'raises a ParserError' do
+        artifact = double(:artifact, job_id: 1)
+
+        expect { parser.build_annotation_rows(artifact, [{}]) }
+          .to raise_error(described_class::ParserError)
+      end
+    end
+
+    context 'when the JSON objects are valid' do
+      it 'returns the database attributes' do
+        artifact = double(:artifact, job_id: 1)
+        annotations = [
+          {
+            'severity' => 'info',
+            'summary' => 'summary here',
+            'description' => 'description here',
+            'line_number' => 14,
+            'file_path' => 'README.md'
+          }
+        ]
+
+        attributes = parser.build_annotation_rows(artifact, annotations)
+
+        expect(attributes).to eq([
+          {
+            build_id: 1,
+            severity: Ci::BuildAnnotation.severities[:info],
+            summary: 'summary here',
+            description: 'description here',
+            line_number: 14,
+            file_path: 'README.md'
+          }
+        ])
+      end
+    end
+  end
+
+  describe '#validate_database_attributes!' do
+    context 'when the attributes are invalid' do
+      it 'raises a ParserError' do
+        annotation = {
+          'severity' => 'info',
+          'summary' => 'summary here',
+          'description' => 'description here',
+          'line_number' => 14
+        }
+
+        attributes = {
+          build_id: 1,
+          severity: Ci::BuildAnnotation.severities[:info],
+          summary: 'summary here',
+          description: 'description here',
+          line_number: 14
+        }
+
+        expect { parser.validate_database_attributes!(annotation, attributes) }
+          .to raise_error(described_class::ParserError)
+      end
+    end
+
+    context 'when the attributes are valid' do
+      it 'returns nil' do
+        annotation = {
+          'severity' => 'info',
+          'summary' => 'summary here',
+          'description' => 'description here'
+        }
+
+        attributes = {
+          build_id: 1,
+          severity: Ci::BuildAnnotation.severities[:info],
+          summary: 'summary here',
+          description: 'description here'
+        }
+
+        expect(parser.validate_database_attributes!(annotation, attributes))
+          .to be_nil
+      end
+    end
+
+    it 'does not execute any SQL queries' do
+      annotation = {
+        'severity' => 'info',
+        'summary' => 'summary here',
+        'description' => 'description here',
+        'line_number' => 14,
+        'file_path' => 'README.md'
+      }
+
+      attributes = {
+        build_id: 1,
+        severity: Ci::BuildAnnotation.severities[:info],
+        summary: 'summary here',
+        description: 'description here',
+        line_number: 14,
+        file_path: 'README.md'
+      }
+
+      # It's possible that in a rare case this test is the very first test to
+      # run. This means the schema for `Ci::BuildAnnotation` might not be loaded
+      # yet, which would cause the below assertion to fail.
+      #
+      # To prevent this from happening, we ensure the schema is always present
+      # before testing the number of queries executed during validation.
+      Ci::BuildAnnotation.columns
+
+      queries = ActiveRecord::QueryRecorder
+        .new { parser.validate_database_attributes!(annotation, attributes) }
+        .count
+
+      expect(queries).to be_zero
+    end
+  end
+
+  describe '#parser_error!' do
+    it 'raises a ParserError' do
+      expect { parser.parser_error!({}, 'foo') }.to raise_error(
+        described_class::ParserError,
+        'The annotation {} is invalid: foo'
+      )
+    end
+  end
+end
diff --git a/spec/models/ci/build_annotation_spec.rb b/spec/models/ci/build_annotation_spec.rb
new file mode 100644
index 00000000000..7b716be48ab
--- /dev/null
+++ b/spec/models/ci/build_annotation_spec.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::BuildAnnotation do
+  def annotation(*args)
+    described_class.new(*args).tap do |annotation|
+      annotation.validate
+    end
+  end
+
+  describe 'when validating the severity' do
+    it 'produces an error if it is missing' do
+      expect(annotation.errors[:severity]).not_to be_empty
+    end
+  end
+
+  describe 'when validating the CI build ID' do
+    it 'produces an error if it is missing' do
+      expect(annotation.errors[:build_id]).not_to be_empty
+    end
+  end
+
+  describe 'when validating the summary' do
+    it 'produces an error if it is missing' do
+      expect(annotation.errors[:summary]).not_to be_empty
+    end
+
+    it 'produces an error if it exceeds the maximum length' do
+      expect(annotation(summary: 'a' * 1024).errors[:summary]).not_to be_empty
+    end
+
+    it 'does not produce an error if it is valid' do
+      expect(annotation(summary: 'a').errors[:summary]).to be_empty
+    end
+  end
+
+  describe 'when validating the line number' do
+    it 'produces an error if it is zero' do
+      expect(annotation(line_number: 0).errors[:line_number]).not_to be_empty
+    end
+
+    it 'produces an error if it is negative' do
+      expect(annotation(line_number: -1).errors[:line_number]).not_to be_empty
+    end
+
+    it 'produces an error if it is too great' do
+      expect(annotation(line_number: 40_000).errors[:line_number])
+        .not_to be_empty
+    end
+
+    it 'produces an error if the file path is not present' do
+      expect(annotation(line_number: 1).errors[:file_path]).not_to be_empty
+    end
+
+    it 'does not produce an error if it is valid' do
+      row = annotation(line_number: 1, file_path: 'foo.rb')
+
+      expect(row.errors[:line_number]).to be_empty
+      expect(row.errors[:file_path]).to be_empty
+    end
+
+    it 'does not produce an error if it and the file path are not given' do
+      row = annotation
+
+      expect(row.errors[:line_number]).to be_empty
+      expect(row.errors[:file_path]).to be_empty
+    end
+  end
+
+  describe 'when validating the file path' do
+    it 'produces an error if the line number is not present' do
+      expect(annotation(file_path: 'foo.rb').errors[:line_number])
+        .not_to be_empty
+    end
+
+    it 'does not produce an error if it is valid' do
+      row = annotation(line_number: 1, file_path: 'foo.rb')
+
+      expect(row.errors[:file_path]).to be_empty
+      expect(row.errors[:line_number]).to be_empty
+    end
+  end
+end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 8a1bbb26e57..0550b456c21 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -3602,4 +3602,49 @@ describe Ci::Build do
       end
     end
   end
+
+  describe '#first_build_annotation_artifact' do
+    context 'when there are no build annotations' do
+      it 'returns nil' do
+        build = create(:ci_build)
+
+        expect(build.first_build_annotation_artifact).to be_nil
+      end
+    end
+
+    context 'when there is one build annotation artifact' do
+      it 'returns the artifact' do
+        build = create(:ci_build)
+        artifact = create(
+          :ci_job_artifact,
+          job: build,
+          file_type: :annotation,
+          file_format: :gzip
+        )
+
+        expect(build.first_build_annotation_artifact).to eq(artifact)
+      end
+    end
+
+    context 'when there are multiple build annotation artifacts' do
+      it 'returns the oldest artifact' do
+        build = create(:ci_build)
+        artifact1 = create(
+          :ci_job_artifact,
+          job: build,
+          file_type: :annotation,
+          file_format: :gzip
+        )
+
+        create(
+          :ci_job_artifact,
+          job: build,
+          file_type: :annotation,
+          file_format: :gzip
+        )
+
+        expect(build.first_build_annotation_artifact).to eq(artifact1)
+      end
+    end
+  end
 end
diff --git a/spec/services/ci/create_build_annotations_service_spec.rb b/spec/services/ci/create_build_annotations_service_spec.rb
new file mode 100644
index 00000000000..032ee25745d
--- /dev/null
+++ b/spec/services/ci/create_build_annotations_service_spec.rb
@@ -0,0 +1,118 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::CreateBuildAnnotationsService do
+  describe '#execute' do
+    let(:ci_build) { create(:ci_build) }
+    let(:service) { described_class.new(ci_build) }
+
+    context 'when there is no annotations artifact' do
+      it 'does nothing' do
+        service.execute
+
+        expect(Ci::BuildAnnotation.count).to be_zero
+      end
+    end
+
+    context 'when there is an annotation artifact' do
+      it 'imports the artifact' do
+        artifact = double(:artifact, job_id: ci_build.id, size: 1)
+        json = JSON.dump([{ severity: 'warning', summary: 'hello' }])
+
+        allow(artifact)
+          .to receive(:each_blob)
+          .and_yield(json)
+
+        allow(ci_build)
+          .to receive(:first_build_annotation_artifact)
+          .and_return(artifact)
+
+        service.execute
+
+        expect(Ci::BuildAnnotation.count).to eq(1)
+      end
+    end
+  end
+
+  describe '#parse_annotations' do
+    let(:service) { described_class.new(double(:build, id: 1)) }
+
+    context 'when the annotations are valid' do
+      it 'returns the database rows to insert' do
+        artifact = double(:artifact, job_id: 1, size: 1)
+        json = JSON.dump([{ severity: 'warning', summary: 'hello' }])
+
+        allow(artifact)
+          .to receive(:each_blob)
+          .and_yield(json)
+
+        expect(service.parse_annotations(artifact)).to eq([
+          {
+            build_id: 1,
+            severity: Ci::BuildAnnotation.severities[:warning],
+            summary: 'hello',
+            description: nil,
+            line_number: nil,
+            file_path: nil
+          }
+        ])
+      end
+    end
+
+    context 'when the annotations are not valid' do
+      it 'returns an error row to insert' do
+        artifact = double(:artifact, job_id: 1, size: 1)
+        json = JSON.dump([{ summary: 'hello' }])
+
+        allow(artifact)
+          .to receive(:each_blob)
+          .and_yield(json)
+
+        annotations = service.parse_annotations(artifact)
+
+        expect(annotations.length).to eq(1)
+
+        expect(annotations[0][:severity])
+          .to eq(Ci::BuildAnnotation.severities[:error])
+
+        expect(annotations[0][:summary]).to be_an_instance_of(String)
+      end
+    end
+  end
+
+  describe '#insert_annotations' do
+    it 'inserts the build annotations into the database' do
+      build = create(:ci_build)
+      row = {
+        build_id: build.id,
+        severity: Ci::BuildAnnotation.severities[:warning],
+        summary: 'hello',
+        description: nil,
+        line_number: nil,
+        file_path: nil
+      }
+
+      service = described_class.new(build)
+
+      service.insert_annotations([row])
+
+      expect(Ci::BuildAnnotation.count).to eq(1)
+    end
+  end
+
+  describe '#build_error_annotation' do
+    it 'returns an annotation to use when an error was produced' do
+      build = create(:ci_build)
+      service = described_class.new(build)
+
+      expect(service.build_error_annotation('foo')).to eq([
+        {
+          build_id: build.id,
+          severity: Ci::BuildAnnotation.severities[:error],
+          summary: 'foo'
+        }
+      ])
+    end
+  end
+end
diff --git a/spec/workers/ci/create_build_annotations_worker_spec.rb b/spec/workers/ci/create_build_annotations_worker_spec.rb
new file mode 100644
index 00000000000..63ffba12c6f
--- /dev/null
+++ b/spec/workers/ci/create_build_annotations_worker_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::CreateBuildAnnotationsWorker do
+  describe '#perform' do
+    context 'when the CI build does not exist' do
+      it 'does nothing' do
+        expect(Ci::CreateBuildAnnotationsService)
+          .not_to receive(:new)
+
+        described_class.new.perform(-1)
+      end
+    end
+
+    context 'when the CI build exists' do
+      it 'creates the build annotations for the build' do
+        build = create(:ci_build)
+        worker = described_class.new
+        service = instance_spy(Ci::CreateBuildAnnotationsService)
+
+        allow(Ci::CreateBuildAnnotationsService)
+          .to receive(:new)
+          .with(build)
+          .and_return(service)
+
+        allow(service)
+          .to receive(:execute)
+
+        worker.perform(build.id)
+
+        expect(service)
+          .to have_received(:execute)
+      end
+    end
+  end
+end