diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 72e646210a8..6ca46ae89c1 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -267,24 +267,7 @@ module Ci end def parse_trace_sections! - return false unless trace_sections.empty? - - sections = trace.extract_sections.map do |attr| - name = attr.delete(:name) - name_record = begin - project.build_trace_section_names.find_or_create_by!(name: name) - rescue ActiveRecord::RecordInvalid - project.build_trace_section_names.find_by!(name: name) - end - - attr.merge( - build_id: self.id, - project_id: self.project_id, - section_name_id: name_record.id) - end - - Gitlab::Database.bulk_insert(Ci::BuildTraceSection.table_name, sections) - true + ExtractSectionsFromBuildTraceService.new(project, user).execute(self) end def trace diff --git a/app/services/ci/extract_sections_from_build_trace_service.rb b/app/services/ci/extract_sections_from_build_trace_service.rb new file mode 100644 index 00000000000..75f9e0f897d --- /dev/null +++ b/app/services/ci/extract_sections_from_build_trace_service.rb @@ -0,0 +1,30 @@ +module Ci + class ExtractSectionsFromBuildTraceService < BaseService + def execute(build) + return false unless build.trace_sections.empty? + + Gitlab::Database.bulk_insert(BuildTraceSection.table_name, extract_sections(build)) + true + end + + private + + def find_or_create_name(name) + project.build_trace_section_names.find_or_create_by!(name: name) + rescue ActiveRecord::RecordInvalid + project.build_trace_section_names.find_by!(name: name) + end + + def extract_sections(build) + build.trace.extract_sections.map do |attr| + name = attr.delete(:name) + name_record = find_or_create_name(name) + + attr.merge( + build_id: build.id, + project_id: project.id, + section_name_id: name_record.id) + end + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 7fe12f92a2a..06f76b5501e 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -322,24 +322,13 @@ describe Ci::Build do end describe '#parse_trace_sections!' do - context "when the build trace has sections markers," do - before do - build.trace.set(File.read(expand_fixture_path('trace/trace_with_sections'))) - end + it 'calls ExtractSectionsFromBuildTraceService' do + expect(Ci::ExtractSectionsFromBuildTraceService) + .to receive(:new).with(project, build.user).once.and_call_original + expect_any_instance_of(Ci::ExtractSectionsFromBuildTraceService) + .to receive(:execute).with(build).once - it "saves the correct extracted sections" do - expect(build.trace_sections).to be_empty - expect(build.parse_trace_sections!).to be(true) - expect(build.trace_sections).not_to be_empty - end - - it "fails if trace_sections isn't empty" do - expect(build.parse_trace_sections!).to be(true) - expect(build.trace_sections).not_to be_empty - - expect(build.parse_trace_sections!).to be(false) - expect(build.trace_sections).not_to be_empty - end + build.parse_trace_sections! end end diff --git a/spec/services/ci/extract_sections_from_build_trace_service_spec.rb b/spec/services/ci/extract_sections_from_build_trace_service_spec.rb new file mode 100644 index 00000000000..28f2fa7903a --- /dev/null +++ b/spec/services/ci/extract_sections_from_build_trace_service_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Ci::ExtractSectionsFromBuildTraceService, '#execute' do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:build) { create(:ci_build, project: project) } + + subject { described_class.new(project, user) } + + shared_examples 'build trace has sections markers' do + before do + build.trace.set(File.read(expand_fixture_path('trace/trace_with_sections'))) + end + + it 'saves the correct extracted sections' do + expect(build.trace_sections).to be_empty + expect(subject.execute(build)).to be(true) + expect(build.trace_sections).not_to be_empty + end + + it "fails if trace_sections isn't empty" do + expect(subject.execute(build)).to be(true) + expect(build.trace_sections).not_to be_empty + + expect(subject.execute(build)).to be(false) + expect(build.trace_sections).not_to be_empty + end + end + + shared_examples 'build trace has no sections markers' do + before do + build.trace.set('no markerts') + end + + it 'extracts no sections' do + expect(build.trace_sections).to be_empty + expect(subject.execute(build)).to be(true) + expect(build.trace_sections).to be_empty + end + end + + context 'when the build has no user' do + it_behaves_like 'build trace has sections markers' + it_behaves_like 'build trace has no sections markers' + end + + context 'when the build has a valid user' do + before do + build.user = user + end + + it_behaves_like 'build trace has sections markers' + it_behaves_like 'build trace has no sections markers' + end +end