Class method YamlProcessor.new_with_validation_errors

- Moved new structured validations messaging logic into a class method
  wrapping .new to prevent needing to process the YAML twice.
- Should also make it easier for the application to switch over to
  using structured error messaging by directly using this class methid
  instead of a regular YamlProcessor.new

app/controllers/projects/ci/lints_controller.rb

@@ -8,11 +8,13 @@ class Projects::Ci::LintsController < Projects::ApplicationController
 
   def create
     @content = params[:content]
-    @error  = Gitlab::Ci::YamlProcessor.validation_errors(@content, yaml_processor_options).join(', ')
-    @status = @error.blank?
+    result   = Gitlab::Ci::YamlProcessor.new_with_validation_errors(@content, yaml_processor_options)
 
-    if @error.blank?
-      @config_processor = Gitlab::Ci::YamlProcessor.new(@content, yaml_processor_options)
+    @error   = result.errors.join(', ')
+    @status  = result.valid?
+
+    if result.valid?
+      @config_processor = result.content
       @stages = @config_processor.stages
       @builds = @config_processor.builds
       @jobs = @config_processor.jobs

lib/gitlab/ci/yaml_processor.rb

@@ -9,6 +9,12 @@ module Gitlab
 
       attr_reader :stages, :jobs
 
+      ResultWithErrors = Struct.new(:content, :errors) do
+        def valid?
+          errors.empty?
+        end
+      end
+
       def initialize(config, opts = {})
         @ci_config = Gitlab::Ci::Config.new(config, **opts)
         @config = @ci_config.to_hash
@@ -22,6 +28,18 @@ module Gitlab
         raise ValidationError, e.message
       end
 
+      def self.new_with_validation_errors(content, opts = {})
+        return ResultWithErrors.new('', ['Please provide content of .gitlab-ci.yml']) if content.blank?
+
+        config = Gitlab::Ci::Config.new(content, **opts)
+        return ResultWithErrors.new("", config.errors) unless config.valid?
+
+        config = Gitlab::Ci::YamlProcessor.new(content, opts)
+        ResultWithErrors.new(config, [])
+      rescue ValidationError, Gitlab::Ci::Config::ConfigError => e
+        ResultWithErrors.new('', [e.message])
+      end
+
       def builds
         @jobs.map do |name, _|
           build_attributes(name)
@@ -102,18 +120,6 @@ module Gitlab
         end
       end
 
-      def self.validation_errors(content, opts = {})
-        return ['Please provide content of .gitlab-ci.yml'] if content.blank?
-
-        config = Gitlab::Ci::Config.new(content, **opts)
-        return config.errors unless config.valid?
-
-        Gitlab::Ci::YamlProcessor.new(content, opts)
-        []
-      rescue ValidationError, Gitlab::Ci::Config::ConfigError => e
-        [e.message]
-      end
-
       private
 
       def initial_parsing

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -2236,43 +2236,67 @@ module Gitlab
         end
       end
 
-      describe '#validation_errors' do
-        subject { Gitlab::Ci::YamlProcessor.validation_errors(content) }
+      describe '.new_with_validation_errors' do
+        subject { Gitlab::Ci::YamlProcessor.new_with_validation_errors(content) }
 
         context 'when the YAML could not be parsed' do
           let(:content) { YAML.dump('invalid: yaml: test') }
 
-          it { is_expected.to eq(['Invalid configuration format']) }
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['Invalid configuration format'])
+            expect(subject.content).to be_blank
+          end
         end
 
         context 'when the tags parameter is invalid' do
           let(:content) { YAML.dump({ rspec: { script: 'test', tags: 'mysql' } }) }
 
-          it { is_expected.to eq(['jobs:rspec:tags config should be an array of strings']) }
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['jobs:rspec:tags config should be an array of strings'])
+            expect(subject.content).to be_blank
+          end
         end
 
         context 'when the configuration contains multiple keyword-syntax errors' do
           let(:content) { YAML.dump({ rspec: { script: 'test', bad_tags: 'mysql', rules: { wrong: 'format' } } }) }
 
-          it { is_expected.to eq(['jobs:rspec config contains unknown keys: bad_tags', 'jobs:rspec rules should be an array of hashes']) }
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['jobs:rspec config contains unknown keys: bad_tags', 'jobs:rspec rules should be an array of hashes'])
+            expect(subject.content).to be_blank
+          end
         end
 
         context 'when YAML content is empty' do
           let(:content) { '' }
 
-          it { is_expected.to eq(['Please provide content of .gitlab-ci.yml']) }
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['Please provide content of .gitlab-ci.yml'])
+            expect(subject.content).to be_blank
+          end
         end
 
         context 'when the YAML contains an unknown alias' do
           let(:content) { 'steps: *bad_alias' }
 
-          it { is_expected.to eq(['Unknown alias: bad_alias']) }
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['Unknown alias: bad_alias'])
+            expect(subject.content).to be_blank
+          end
         end
 
         context 'when the YAML is valid' do
           let(:content) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) }
 
-          it { is_expected.to be_empty }
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(true)
+            expect(subject.errors).to be_empty
+            expect(subject.content).to be_present
+          end
         end
       end
     end