Extract `Ci::Build#parse_trace_sections!` into a service

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

app/services/ci/extract_sections_from_build_trace_service.rb

+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

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 "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
+    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
+      build.parse_trace_sections!
     end
   end

spec/services/ci/extract_sections_from_build_trace_service_spec.rb

+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