From 2c8b830fdbf749e8bb7461d5c3ce4699b77ce3ca Mon Sep 17 00:00:00 2001 From: Katarzyna Kobierska Date: Mon, 29 Aug 2016 15:07:19 +0200 Subject: Code refactoring --- app/controllers/ci/lints_controller.rb | 9 ++---- config/routes.rb | 3 -- lib/api/lint.rb | 43 ++++++++++++---------------- lib/ci/gitlab_ci_yaml_processor.rb | 16 +++++++---- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 27 ++++++++++++----- spec/requests/api/lint_spec.rb | 31 ++++++++++---------- 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') -- cgit v1.2.3