From 910bf96ec3d60194b2fe4444c1df24f141b8450b Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 14 Sep 2015 18:14:17 +0300 Subject: [PATCH] fix specs. Stage 2 --- lib/api/helpers.rb | 5 +- lib/ci/api/api.rb | 2 + lib/ci/api/helpers.rb | 89 ++-------------------------- spec/requests/ci/api/runners_spec.rb | 26 ++++---- spec/support/api_helpers.rb | 11 ++++ 5 files changed, 32 insertions(+), 101 deletions(-) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 76c9cc2e3a4..ef0f897a2fb 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -148,15 +148,14 @@ module API end end - def attributes_for_keys(keys) + def attributes_for_keys(keys, custom_params = nil) + params_hash = custom_params || params attrs = {} - keys.each do |key| if params[key].present? or (params.has_key?(key) and params[key] == false) attrs[key] = params[key] end end - ActionController::Parameters.new(attrs).permit! end diff --git a/lib/ci/api/api.rb b/lib/ci/api/api.rb index 392fb548001..172c6f22164 100644 --- a/lib/ci/api/api.rb +++ b/lib/ci/api/api.rb @@ -3,6 +3,7 @@ Dir["#{Rails.root}/lib/ci/api/*.rb"].each {|file| require file} module Ci module API class API < Grape::API + include APIGuard version 'v1', using: :path rescue_from ActiveRecord::RecordNotFound do @@ -25,6 +26,7 @@ module Ci format :json helpers Helpers + helpers ::API::APIHelpers mount Builds mount Commits diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index 3f58670fb49..9197f917d73 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -1,30 +1,6 @@ module Ci module API module Helpers - PRIVATE_TOKEN_PARAM = :private_token - PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN" - ACCESS_TOKEN_PARAM = :access_token - ACCESS_TOKEN_HEADER = "HTTP_ACCESS_TOKEN" - UPDATE_RUNNER_EVERY = 60 - - def current_user - @current_user ||= begin - options = { - access_token: (params[ACCESS_TOKEN_PARAM] || env[ACCESS_TOKEN_HEADER]), - private_token: (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]), - } - Ci::UserSession.new.authenticate(options.compact) - end - end - - def current_runner - @runner ||= Ci::Runner.find_by_token(params[:token].to_s) - end - - def authenticate! - forbidden! unless current_user - end - def authenticate_runners! forbidden! unless params[:token] == GitlabCi::REGISTRATION_TOKEN end @@ -43,72 +19,15 @@ module Ci end end + def current_runner + @runner ||= Runner.find_by_token(params[:token].to_s) + end + def update_runner_info return unless params["info"].present? info = attributes_for_keys(["name", "version", "revision", "platform", "architecture"], params["info"]) current_runner.update(info) end - - # Checks the occurrences of required attributes, each attribute must be present in the params hash - # or a Bad Request error is invoked. - # - # Parameters: - # keys (required) - A hash consisting of keys that must be present - def required_attributes!(keys) - keys.each do |key| - bad_request!(key) unless params[key].present? - end - end - - def attributes_for_keys(keys, custom_params = nil) - params_hash = custom_params || params - attrs = {} - keys.each do |key| - attrs[key] = params_hash[key] if params_hash[key].present? - end - attrs - end - - # error helpers - - def forbidden! - render_api_error!('403 Forbidden', 403) - end - - def bad_request!(attribute) - message = ["400 (Bad request)"] - message << "\"" + attribute.to_s + "\" not given" - render_api_error!(message.join(' '), 400) - end - - def not_found!(resource = nil) - message = ["404"] - message << resource if resource - message << "Not Found" - render_api_error!(message.join(' '), 404) - end - - def unauthorized! - render_api_error!('401 Unauthorized', 401) - end - - def not_allowed! - render_api_error!('Method Not Allowed', 405) - end - - def render_api_error!(message, status) - error!({ 'message' => message }, status) - end - - private - - def abilities - @abilities ||= begin - abilities = Six.new - abilities << Ability - abilities - end - end end end end diff --git a/spec/requests/ci/api/runners_spec.rb b/spec/requests/ci/api/runners_spec.rb index 714e5a5a84f..11dc089e1f5 100644 --- a/spec/requests/ci/api/runners_spec.rb +++ b/spec/requests/ci/api/runners_spec.rb @@ -9,8 +9,8 @@ describe Ci::API::API do end describe "GET /runners" do - let(:gitlab_url) { GitlabCi.config.gitlab_server.url } - let(:private_token) { Network.new.authenticate(access_token: "some_token")["private_token"] } + let(:gitlab_url) { GitlabCi.config.gitlab_ci.url } + let(:private_token) { create(:user).private_token } let(:options) do { private_token: private_token, @@ -23,7 +23,7 @@ describe Ci::API::API do end it "should retrieve a list of all runners" do - get api("/runners"), options + get ci_api("/runners", nil), options expect(response.status).to eq(200) expect(json_response.count).to eq(5) expect(json_response.last).to have_key("id") @@ -33,41 +33,41 @@ describe Ci::API::API do describe "POST /runners/register" do describe "should create a runner if token provided" do - before { post api("/runners/register"), token: GitlabCi::REGISTRATION_TOKEN } + before { post ci_api("/runners/register"), token: GitlabCi::REGISTRATION_TOKEN } it { expect(response.status).to eq(201) } end describe "should create a runner with description" do - before { post api("/runners/register"), token: GitlabCi::REGISTRATION_TOKEN, description: "server.hostname" } + before { post ci_api("/runners/register"), token: GitlabCi::REGISTRATION_TOKEN, description: "server.hostname" } it { expect(response.status).to eq(201) } - it { expect(Runner.first.description).to eq("server.hostname") } + it { expect(Ci::Runner.first.description).to eq("server.hostname") } end describe "should create a runner with tags" do - before { post api("/runners/register"), token: GitlabCi::REGISTRATION_TOKEN, tag_list: "tag1, tag2" } + before { post ci_api("/runners/register"), token: GitlabCi::REGISTRATION_TOKEN, tag_list: "tag1, tag2" } it { expect(response.status).to eq(201) } - it { expect(Runner.first.tag_list.sort).to eq(["tag1", "tag2"]) } + it { expect(Ci::Runner.first.tag_list.sort).to eq(["tag1", "tag2"]) } end describe "should create a runner if project token provided" do let(:project) { FactoryGirl.create(:ci_project) } - before { post api("/runners/register"), token: project.token } + before { post ci_api("/runners/register"), token: project.token } it { expect(response.status).to eq(201) } it { expect(project.runners.size).to eq(1) } end it "should return 403 error if token is invalid" do - post api("/runners/register"), token: 'invalid' + post ci_api("/runners/register"), token: 'invalid' expect(response.status).to eq(403) end it "should return 400 error if no token" do - post api("/runners/register") + post ci_api("/runners/register") expect(response.status).to eq(400) end @@ -75,9 +75,9 @@ describe Ci::API::API do describe "DELETE /runners/delete" do let!(:runner) { FactoryGirl.create(:ci_runner) } - before { delete api("/runners/delete"), token: runner.token } + before { delete ci_api("/runners/delete"), token: runner.token } it { expect(response.status).to eq(200) } - it { expect(Runner.count).to eq(0) } + it { expect(Ci::Runner.count).to eq(0) } end end diff --git a/spec/support/api_helpers.rb b/spec/support/api_helpers.rb index f63322776d4..1b3cafb497c 100644 --- a/spec/support/api_helpers.rb +++ b/spec/support/api_helpers.rb @@ -28,6 +28,17 @@ module ApiHelpers "&private_token=#{user.private_token}" : "") end + def ci_api(path, user = nil) + "/ci/api/v1/#{path}" + + + # Normalize query string + (path.index('?') ? '' : '?') + + + # Append private_token if given a User object + (user.respond_to?(:private_token) ? + "&private_token=#{user.private_token}" : "") + end + def json_response @_json_response ||= JSON.parse(response.body) end -- GitLab