Code refactoring

6 files changed, 67 insertions, 62 deletions

diff --git a/app/controllers/ci/lints_controller.rb b/app/controllers/ci/lints_controller.rb
index fa57e6c5e14..62f6b23faba 100644
--- a/app/controllers/ci/lints_controller.rb
+++ b/app/controllers/ci/lints_controller.rb
@@ -7,13 +7,10 @@ module Ci
 
     def create
       @content = params[:content]
+      @error = Ci::GitlabCiYamlProcessor.validation_message(@content)
 
-      if @content.blank?
-        @status = false
-        @error = "Please provide content of .gitlab-ci.yml"
-      elsif !Ci::GitlabCiYamlProcessor.errors(@content).nil?
-        @status = false
-        @error = Ci::GitlabCiYamlProcessor.errors(@content)
+      unless @error.blank?
+        @status = @error.blank?
       else
         @config_processor = Ci::GitlabCiYamlProcessor.new(@content)
         @stages = @config_processor.stages
diff --git a/config/routes.rb b/config/routes.rb
index 0b22923039e..262a174437a 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -81,9 +81,6 @@ Rails.application.routes.draw do
     mount Sidekiq::Web, at: '/admin/sidekiq', as: :sidekiq
   end
 
-  # Lint API
-  resources :lint, only: [:show, :create]
-
   # Health check
   get 'health_check(/:checks)' => 'health_check#index', as: :health_check
 
diff --git a/lib/api/lint.rb b/lib/api/lint.rb
index ff35e948e0c..b1c6f52bccb 100644
--- a/lib/api/lint.rb
+++ b/lib/api/lint.rb
@@ -1,33 +1,26 @@
 module API
   class Lint < Grape::API
-    resource :lint do
-      params do
-        requires :content, type: String, desc: 'Content of .gitlab-ci.yml'
-      end
-
-      desc 'Validation of .gitlab-ci.yml content'
-      post do
-        response = {
-          status: '',
-          error: [],
-          jobs: []
-        }
-
-        if Ci::GitlabCiYamlProcessor.errors(params[:content]).nil?
-          config_processor = Ci::GitlabCiYamlProcessor.new(params[:content])
+    desc 'Validation of .gitlab-ci.yml content'
+    params do
+      requires :content, type: String, desc: 'Content of .gitlab-ci.yml'
+    end
 
-          config_processor.builds.each do |build|
-            response[:jobs].push("#{build[:name]}")
-            response[:status] = 'valid'
-          end
-        else
-          response[:error].push(Ci::GitlabCiYamlProcessor.errors(params[:content]))
-          response[:status] = 'invalid'
-        end
+    post 'ci/lint' do
+      error = Ci::GitlabCiYamlProcessor.validation_message(params[:content])
+      response = {
+        status: '',
+        error: ''
+      }
 
-        status 200
-        response
+      if error.blank?
+        response[:status] = 'valid'
+      else
+        response[:error] = error
+        response[:status] = 'invalid'
       end
+
+      status 200
+      response
     end
   end
 end
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 40c84b93799..c547193ce4c 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -78,12 +78,16 @@ module Ci
       }
     end
 
-    def self.errors(content)
-      begin
-        Ci::GitlabCiYamlProcessor.new(content)
-        nil
-      rescue ValidationError, Psych::SyntaxError => e
-        e.message
+    def self.validation_message(content)
+      if content.blank?
+        'Please provide content of .gitlab-ci.yml'
+      else
+        begin
+          Ci::GitlabCiYamlProcessor.new(content)
+          nil
+        rescue ValidationError, Psych::SyntaxError => e
+          e.message
+        end
       end
     end
 
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index d3a37ba6eb5..21aca9ee39f 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1252,20 +1252,33 @@ EOT
     end
 
     describe "#errors" do
-      describe "Error handling" do
-        it "returns an error if the YAML could not be parsed" do
+      context "when the YAML could not be parsed" do
+        it "returns an error about invalid configutaion" do
           content = YAML.dump("invalid: yaml: test")
-          expect(GitlabCiYamlProcessor.errors(content)).to eq "Invalid configuration format"
+
+          expect(GitlabCiYamlProcessor.validation_message(content)).to eq "Invalid configuration format"
         end
+      end
 
-        it "returns an error if the tags parameter is invalid" do
+      context "when the tags parameter is invalid" do
+        it "returns an error about invalid tags" do
           content = YAML.dump({ rspec: { script: "test", tags: "mysql" } })
-          expect(GitlabCiYamlProcessor.errors(content)).to eq "jobs:rspec tags should be an array of strings"
+
+          expect(GitlabCiYamlProcessor.validation_message(content)).to eq "jobs:rspec tags should be an array of strings"
+        end
+      end
+
+      context "when YMAL content is empty" do
+        it "returns an error about missing content" do
+          expect(GitlabCiYamlProcessor.validation_message('')).to eq "Please provide content of .gitlab-ci.yml"
         end
+      end
 
-        it "does not return any errors when the YAML is valid" do
+      context "when the YAML is valid" do
+        it "does not return any errors" do
           content = File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
-          expect(GitlabCiYamlProcessor.errors(content)).to eq nil
+
+          expect(GitlabCiYamlProcessor.validation_message(content)).to be_nil
         end
       end
     end
diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb
index 8bad57819c8..5f8c2829dfa 100644
--- a/spec/requests/api/lint_spec.rb
+++ b/spec/requests/api/lint_spec.rb
@@ -1,44 +1,45 @@
 require 'spec_helper'
 
-describe API::API do
+describe API::Lint, api: true do
   include ApiHelpers
 
-  let(:yaml_content) do
-    File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
-  end
+  describe 'POST /ci/lint' do
+    let(:yaml_content) do
+      File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
+    end
 
-  describe 'POST /lint' do
     context 'with valid .gitlab-ci.yaml content' do
-      it 'validates the content' do
-        post api('/lint'), { content: yaml_content }
+      it 'passes validation' do
+        post api('/ci/lint'), { content: yaml_content }
 
         expect(response).to have_http_status(200)
         expect(json_response).to be_an Hash
         expect(json_response['status']).to eq('valid')
+        expect(json_response['error']).to eq('')
       end
     end
 
     context 'with an invalid .gitlab_ci.yml' do
-      it 'validates the content and shows an error message' do
-        post api('/lint'), { content: 'invalid content' }
+      it 'responds with errors about invalid syntax' do
+        post api('/ci/lint'), { content: 'invalid content' }
 
         expect(response).to have_http_status(200)
         expect(json_response['status']).to eq('invalid')
-        expect(json_response['error']).to eq(['Invalid configuration format'])
+        expect(json_response['error']).to eq('Invalid configuration format')
       end
 
-      it "validates the content and shows a configuration error" do
-        post api('/lint'), { content: '{ image: "ruby:2.1",  services: ["postgres"] }' }
+      it "responds with errors about invalid configuration" do
+        post api('/ci/lint'), { content: '{ image: "ruby:2.1",  services: ["postgres"] }' }
 
         expect(response).to have_http_status(200)
         expect(json_response['status']).to eq('invalid')
-        expect(json_response['error']).to eq(['jobs config should contain at least one visible job'])
+        expect(json_response['error']).to eq('jobs config should contain at least one visible job')
       end
     end
 
     context 'without the content parameter' do
-      it 'shows an error message' do
-        post api('/lint')
+      it 'responds with validation error about missing content' do
+        post api('/ci/lint')
 
         expect(response).to have_http_status(400)
         expect(json_response['error']).to eq('content is missing')