yaml refactoring

8 files changed, 36 insertions, 73 deletions

diff --git a/app/controllers/lints_controller.rb b/app/controllers/lints_controller.rb
index d1b42f6..33ee206 100644
--- a/app/controllers/lints_controller.rb
+++ b/app/controllers/lints_controller.rb
@@ -7,12 +7,12 @@ class LintsController < ApplicationController
   def create
     if params[:content].blank?
       @status = false
-      @error = "Please provide content of your file"
+      @error = "Please provide content of .gitlab-ci.yml"
     else
       @config_processor = GitlabCiYamlProcessor.new params[:content]
       @status = true
     end
-  rescue Psych::SyntaxError => e
+  rescue GitlabCiYamlProcessor::ValidationError => e
     @error = e.message
     @status = false
   rescue Exception => e
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 1d3f214..46900ab 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -94,12 +94,10 @@ class Commit < ActiveRecord::Base
   def create_builds
     return if skip_ci?
 
-    unless config_processor.valid?
-      save_yaml_error(config_processor.errors.join(",")) and return
-    end
-
     begin
       builds_for_ref = config_processor.builds_for_ref(ref, tag)
+    rescue GitlabCiYamlProcessor::ValidationError => e
+      save_yaml_error(e.message) and return
     rescue Exception => e
       logger.error e.message + "\n" + e.backtrace.join("\n")
       save_yaml_error("Undefined yaml error") and return
@@ -132,12 +130,10 @@ class Commit < ActiveRecord::Base
   def create_deploy_builds
     return if skip_ci?
 
-    unless config_processor.valid?
-      save_yaml_error(config_processor.errors.join(",")) and return
-    end
-
     begin
       deploy_builds_for_ref = config_processor.deploy_builds_for_ref(ref, tag)
+    rescue GitlabCiYamlProcessor::ValidationError => e
+      save_yaml_error(e.message) and return
     rescue Exception => e
       logger.error e.message + "\n" + e.backtrace.join("\n")
       save_yaml_error("Undefined yaml error") and return
diff --git a/app/services/create_commit_service.rb b/app/services/create_commit_service.rb
index de5ece3..df678d4 100644
--- a/app/services/create_commit_service.rb
+++ b/app/services/create_commit_service.rb
@@ -4,8 +4,7 @@ class CreateCommitService
     sha = params[:checkout_sha] || params[:after]
     origin_ref = params[:ref]
     yaml_config = params[:ci_yaml_file] || project.generated_yaml_config
-    config_processor = build_config_processor(yaml_config)
-
+    
     unless origin_ref && sha.present?
       return false
     end
@@ -17,10 +16,6 @@ class CreateCommitService
       return false
     end
 
-    unless config_processor.any_jobs?(ref, origin_ref.start_with?('refs/tags/'))
-      return false
-    end
-
     commit = project.commits.find_by_sha_and_ref(sha, ref)
 
     # Create commit if not exists yet
@@ -54,10 +49,4 @@ class CreateCommitService
 
     commit
   end
-
-  private
-
-  def build_config_processor(config_data)
-    @builds_config ||= GitlabCiYamlProcessor.new(config_data)
-  end
 end
diff --git a/app/views/lints/_create.html.haml b/app/views/lints/_create.html.haml
index 34920d7..454ca4b 100644
--- a/app/views/lints/_create.html.haml
+++ b/app/views/lints/_create.html.haml
@@ -47,6 +47,7 @@
     %b Status:
     syntax is incorrect
     %i.icon-remove.incorrect-syntax
-  Error: #{@error}
+  %b Error:
+  = @error
           
     
\ No newline at end of file
diff --git a/lib/gitlab_ci_yaml_processor.rb b/lib/gitlab_ci_yaml_processor.rb
index 51f412a..6bb1c4b 100644
--- a/lib/gitlab_ci_yaml_processor.rb
+++ b/lib/gitlab_ci_yaml_processor.rb
@@ -1,25 +1,20 @@
 class GitlabCiYamlProcessor
-  attr_reader :before_script, :skip_refs, :errors
+  class ValidationError < StandardError;end
 
-  def initialize(config)
-    @errors = []
-
-    @config = YAML.load(config).deep_symbolize_keys
-    @before_script = @config[:before_script] || []
+  attr_reader :before_script
 
-    @config.delete(:before_script)
+  def initialize(config)
+    @config = YAML.load(config)
 
-    @jobs = @config.select{|key, value| value[:type] != "deploy"}
+    unless @config.is_a? Hash
+      raise ValidationError, "YAML should be a hash"
+    end
 
-    @deploy_jobs = @config.select{|key, value| value[:type] == "deploy"}
+    @config = @config.deep_symbolize_keys
 
-  rescue Exception => e
-    @errors << "Yaml file is invalid"
-  end
+    initial_parsing
 
-  def valid?
     validate!
-    !@errors.any?
   end
 
   def deploy_builds_for_ref(ref, tag = false)
@@ -30,10 +25,6 @@ class GitlabCiYamlProcessor
     builds.select{|build| process?(build[:only], build[:except], ref, tag)}
   end
 
-  def any_jobs?(ref, tag = false)
-    builds_for_ref(ref, tag).any? || deploy_builds_for_ref(ref, tag).any?
-  end
-
   def builds
     @jobs.map do |name, job|
       {
@@ -60,6 +51,13 @@ class GitlabCiYamlProcessor
 
   private
 
+  def initial_parsing
+    @before_script = @config[:before_script] || []
+    @config.delete(:before_script)
+    @jobs = @config.select{|key, value| value[:type] != "deploy"}
+    @deploy_jobs = @config.select{|key, value| value[:type] == "deploy"}
+  end
+
   def process?(only_params, except_params, ref, tag)
     return true if only_params.nil? && except_params.nil?
 
@@ -97,41 +95,34 @@ class GitlabCiYamlProcessor
   end
 
   def validate!
-    unless @config.is_a? Hash
-      @errors << "should be a hash"
-      return false
-    end
-
     @jobs.each do |name, job|
       if job[:tags] && !job[:tags].is_a?(Array)
-        @errors << "#{name} job: tags parameter should be an array"
+        raise ValidationError, "#{name} job: tags parameter should be an array"
       end
 
       if job[:only] && !job[:only].is_a?(Array)
-        @errors << "#{name} job: only parameter should be an array"
+        raise ValidationError, "#{name} job: only parameter should be an array"
       end
 
       if job[:except] && !job[:except].is_a?(Array)
-        @errors << "#{name} job: except parameter should be an array"
+        raise ValidationError, "#{name} job: except parameter should be an array"
       end
     end
 
     @deploy_jobs.each do |name, job|
       if job[:tags] && !job[:tags].is_a?(Array)
-        @errors << "#{name} deploy job: tags parameter should be an array"
+        raise ValidationError, "#{name} deploy job: tags parameter should be an array"
       end
 
       if job[:only] && !job[:only].is_a?(Array)
-        @errors << "#{name} deploy job: only parameter should be an array"
+        raise ValidationError, "#{name} deploy job: only parameter should be an array"
       end
 
       if job[:except] && !job[:except].is_a?(Array)
-        @errors << "#{name} deploy job: except parameter should be an array"
+        raise ValidationError, "#{name} deploy job: except parameter should be an array"
       end
     end
 
     true
-  rescue
-    @errors << "Undefined error"
   end
 end
diff --git a/spec/features/lint_spec.rb b/spec/features/lint_spec.rb
index 1160f24..0b3d4e0 100644
--- a/spec/features/lint_spec.rb
+++ b/spec/features/lint_spec.rb
@@ -23,6 +23,6 @@ describe "Lint" do
     fill_in "content", with: ""
     click_on "Validate"
     page.should have_content("Status: syntax is incorrect")
-    page.should have_content("Error: Please provide content of your file")
+    page.should have_content("Error: Please provide content of .gitlab-ci.yml")
   end
 end
diff --git a/spec/lib/gitlab_ci_yaml_processor_spec.rb b/spec/lib/gitlab_ci_yaml_processor_spec.rb
index b101e33..0bbe5c1 100644
--- a/spec/lib/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/gitlab_ci_yaml_processor_spec.rb
@@ -121,17 +121,14 @@ describe GitlabCiYamlProcessor do
 
   describe "Error handling" do
     it "indicates that object is invalid" do
-      config_processor = GitlabCiYamlProcessor.new("invalid_yaml\n!ccdvlf%612334@@@@")
-
-      config_processor.valid?.should be_false
+      expect{GitlabCiYamlProcessor.new("invalid_yaml\n!ccdvlf%612334@@@@")}.to raise_error(GitlabCiYamlProcessor::ValidationError)
     end
 
     it "returns errors" do
       config = YAML.dump({rspec: {tags: "mysql"}})
-      config_processor = GitlabCiYamlProcessor.new(config)
-
-      config_processor.valid?.should be_false
-      config_processor.errors.should == ["rspec job: tags parameter should be an array"]
+      expect do
+        GitlabCiYamlProcessor.new(config)
+      end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: tags parameter should be an array")
     end
   end
 end
\ No newline at end of file
diff --git a/spec/services/create_commit_service_spec.rb b/spec/services/create_commit_service_spec.rb
index f6255f6..ad6ef8e 100644
--- a/spec/services/create_commit_service_spec.rb
+++ b/spec/services/create_commit_service_spec.rb
@@ -61,17 +61,6 @@ describe CreateCommitService do
         result.should be_persisted
       end
 
-      it "does not create commit if there is no appropriate job nor deploy job" do
-        result = service.execute(project,
-          ref: 'refs/tags/0_1',
-          before: '00000000',
-          after: '31das312',
-          ci_yaml_file: YAML.dump({}),
-          commits: [ { message: "Message" } ]
-        )
-        result.should be_false
-      end
-
       it "creates commit if there is no appropriate job but deploy job has right ref setting" do
         config = YAML.dump({deploy: {deploy: "ls", only: ["0_1"]}})