From 026dc4cb989aca8a2cdc03f6cb393d5308732bff Mon Sep 17 00:00:00 2001 From: Joe Corcoran Date: Thu, 9 Jun 2016 16:21:55 +0200 Subject: [PATCH 1/7] Use travis-settings to manage JSON settings field Since we use repository.settings as a kind of dump for all sorts of settings, some user-facing and some not, this lets us leave the db as it is, but pretend to have separate models for each "kind" of setting. --- Gemfile | 1 + Gemfile.lock | 8 ++++++++ lib/travis/api/v3/models/admin_settings.rb | 5 +++++ lib/travis/api/v3/models/repository.rb | 10 +++++++++- lib/travis/api/v3/models/settings.rb | 13 ++----------- lib/travis/api/v3/models/user_settings.rb | 8 ++++++++ lib/travis/api/v3/services/requests/create.rb | 6 +----- 7 files changed, 34 insertions(+), 17 deletions(-) create mode 100644 lib/travis/api/v3/models/admin_settings.rb create mode 100644 lib/travis/api/v3/models/user_settings.rb diff --git a/Gemfile b/Gemfile index ab1dc3bf..ccf97b60 100644 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,7 @@ gem 'travis-settings', github: 'travis-ci/travis-settings' gem 'travis-sidekiqs', github: 'travis-ci/travis-sidekiqs' gem 'travis-yaml', github: 'travis-ci/travis-yaml' +gem 'travis-settings', github: 'travis-ci/travis-settings' gem 'mustermann', github: 'rkh/mustermann' gem 'sinatra' gem 'sinatra-contrib', require: nil #github: 'sinatra/sinatra-contrib', require: nil diff --git a/Gemfile.lock b/Gemfile.lock index 0dda637b..96104e61 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -53,6 +53,14 @@ GIT activemodel virtus +GIT + remote: git://github.com/travis-ci/travis-settings.git + revision: d510e63b6c6f059cccae141c265e7a0c7236d1fd + specs: + travis-settings (0.0.1) + activemodel + virtus + GIT remote: git://github.com/travis-ci/travis-sidekiqs.git revision: c5d4a4abc6c3737f9c43d3333efb94daa18b9fbb diff --git a/lib/travis/api/v3/models/admin_settings.rb b/lib/travis/api/v3/models/admin_settings.rb new file mode 100644 index 00000000..ee1d9d2a --- /dev/null +++ b/lib/travis/api/v3/models/admin_settings.rb @@ -0,0 +1,5 @@ +module Travis::API::V3 + class Models::AdminSettings < Travis::Settings::Model + attribute :api_builds_rate_limit, Integer + end +end diff --git a/lib/travis/api/v3/models/repository.rb b/lib/travis/api/v3/models/repository.rb index b462db88..1b14bd5f 100644 --- a/lib/travis/api/v3/models/repository.rb +++ b/lib/travis/api/v3/models/repository.rb @@ -65,7 +65,15 @@ module Travis::API::V3 end def settings - @settings ||= JSON.load(super) + @settings ||= JSON.load(super || '{}'.freeze) + end + + def user_settings + @user_settings ||= Models::UserSettings.new(settings) + end + + def admin_settings + @admin_settings ||= Models::AdminSettings.new(settings) end end end diff --git a/lib/travis/api/v3/models/settings.rb b/lib/travis/api/v3/models/settings.rb index 00949495..0d3e4593 100644 --- a/lib/travis/api/v3/models/settings.rb +++ b/lib/travis/api/v3/models/settings.rb @@ -1,12 +1,5 @@ module Travis::API::V3 class Models::Settings - DEFAULTS = { - 'builds_only_with_travis_yml' => false, - 'build_pushes' => true, - 'build_pull_requests' => true, - 'maximum_number_of_builds' => 0 - }.freeze - attr_reader :repository def initialize(repository) @@ -14,13 +7,11 @@ module Travis::API::V3 end def to_h - DEFAULTS.merge(repository.settings || {}) + repository.user_settings.to_hash end def update(settings = {}) - settings = to_h.merge(settings) - repository.settings.clear - settings.each { |k, v| repository.settings[k] = v } + repository.user_settings.update(settings) repository.save! end end diff --git a/lib/travis/api/v3/models/user_settings.rb b/lib/travis/api/v3/models/user_settings.rb new file mode 100644 index 00000000..42e6519e --- /dev/null +++ b/lib/travis/api/v3/models/user_settings.rb @@ -0,0 +1,8 @@ +module Travis::API::V3 + class Models::UserSettings < Travis::Settings::Model + attribute :builds_only_with_travis_yml, Boolean, default: false + attribute :build_pushes, Boolean, default: true + attribute :build_pull_requests, Boolean, default: true + attribute :maximum_number_of_builds, Integer, default: 0 + end +end diff --git a/lib/travis/api/v3/services/requests/create.rb b/lib/travis/api/v3/services/requests/create.rb index ea3827de..87c54d2d 100644 --- a/lib/travis/api/v3/services/requests/create.rb +++ b/lib/travis/api/v3/services/requests/create.rb @@ -23,11 +23,7 @@ module Travis::API::V3 end def limit(repository) - if repository.settings.nil? - Travis.config.requests_create_api_limit || LIMIT - else - repository.settings["api_builds_rate_limit"] || Travis.config.requests_create_api_limit || LIMIT - end + repository.admin_settings.api_builds_rate_limit || Travis.config.requests_create_api_limit || LIMIT end def remaining_requests(repository) From cae6da540fabb42fd83d6e03b86277bc5a86ada9 Mon Sep 17 00:00:00 2001 From: Joe Corcoran Date: Tue, 14 Jun 2016 11:59:15 +0200 Subject: [PATCH 2/7] Add env vars endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds API V3 endpoints for querying, creating, modifying and deleting environment variables. It has no concept of encryption yet and should be considered a work in progress. We should also talk about the slightly off-track approach in the EnvVars::Create service – maybe there's a way to standardise the querying and rendering for post requests? :coffee: --- lib/travis/api/v3.rb | 1 + lib/travis/api/v3/access_control/generic.rb | 8 +++ lib/travis/api/v3/models/env_var.rb | 18 +++++ lib/travis/api/v3/models/env_vars.rb | 11 +++ lib/travis/api/v3/models/repository.rb | 6 ++ lib/travis/api/v3/queries/env_var.rb | 21 ++++++ lib/travis/api/v3/queries/env_vars.rb | 27 ++++++++ lib/travis/api/v3/renderer/env_var.rb | 5 ++ lib/travis/api/v3/renderer/env_vars.rb | 6 ++ lib/travis/api/v3/routes.rb | 13 ++++ lib/travis/api/v3/services.rb | 2 + lib/travis/api/v3/services/env_var/delete.rb | 12 ++++ lib/travis/api/v3/services/env_var/find.rb | 12 ++++ lib/travis/api/v3/services/env_var/update.rb | 12 ++++ lib/travis/api/v3/services/env_vars/create.rb | 13 ++++ .../v3/services/env_vars/for_repository.rb | 9 +++ spec/spec_helper.rb | 34 ++++++++++ spec/support/shared_examples.rb | 34 ++++++++++ spec/v3/services/env_var/delete_spec.rb | 33 +++++++++ spec/v3/services/env_var/find_spec.rb | 43 ++++++++++++ spec/v3/services/env_var/update_spec.rb | 50 ++++++++++++++ spec/v3/services/env_vars/create_spec.rb | 67 +++++++++++++++++++ .../services/env_vars/for_repository_spec.rb | 59 ++++++++++++++++ spec/v3/services/settings_spec.rb | 9 +-- 24 files changed, 497 insertions(+), 8 deletions(-) create mode 100644 lib/travis/api/v3/models/env_var.rb create mode 100644 lib/travis/api/v3/models/env_vars.rb create mode 100644 lib/travis/api/v3/queries/env_var.rb create mode 100644 lib/travis/api/v3/queries/env_vars.rb create mode 100644 lib/travis/api/v3/renderer/env_var.rb create mode 100644 lib/travis/api/v3/renderer/env_vars.rb create mode 100644 lib/travis/api/v3/services/env_var/delete.rb create mode 100644 lib/travis/api/v3/services/env_var/find.rb create mode 100644 lib/travis/api/v3/services/env_var/update.rb create mode 100644 lib/travis/api/v3/services/env_vars/create.rb create mode 100644 lib/travis/api/v3/services/env_vars/for_repository.rb create mode 100644 spec/support/shared_examples.rb create mode 100644 spec/v3/services/env_var/delete_spec.rb create mode 100644 spec/v3/services/env_var/find_spec.rb create mode 100644 spec/v3/services/env_var/update_spec.rb create mode 100644 spec/v3/services/env_vars/create_spec.rb create mode 100644 spec/v3/services/env_vars/for_repository_spec.rb diff --git a/lib/travis/api/v3.rb b/lib/travis/api/v3.rb index 610de9cd..f05aec5e 100644 --- a/lib/travis/api/v3.rb +++ b/lib/travis/api/v3.rb @@ -31,6 +31,7 @@ module Travis AlreadySyncing = ClientError .create('sync already in progress', status: 409) BuildAlreadyRunning = ClientError .create('build already running, cannot restart', status: 409) BuildNotCancelable = ClientError .create('build is not running, cannot cancel', status: 409) + DuplicateResource = ClientError .create('resource already exists', status: 409) EntityMissing = NotFound .create(type: 'not_found') InsufficientAccess = ClientError .create(status: 403) JobAlreadyRunning = ClientError .create('job already running, cannot restart', status: 409) diff --git a/lib/travis/api/v3/access_control/generic.rb b/lib/travis/api/v3/access_control/generic.rb index 92b68e1e..f3671da7 100644 --- a/lib/travis/api/v3/access_control/generic.rb +++ b/lib/travis/api/v3/access_control/generic.rb @@ -96,6 +96,14 @@ module Travis::API::V3 repository_visible?(settings.repository) end + def env_vars_visible?(env_vars) + repository_visible?(env_vars.repository) + end + + def env_var_visible?(env_var) + repository_visible?(env_var.repository) + end + def private_repository_visible?(repository) false end diff --git a/lib/travis/api/v3/models/env_var.rb b/lib/travis/api/v3/models/env_var.rb new file mode 100644 index 00000000..1d4c7f8f --- /dev/null +++ b/lib/travis/api/v3/models/env_var.rb @@ -0,0 +1,18 @@ +module Travis::API::V3 + class Models::EnvVar < Travis::Settings::Model + attribute :id, Integer + attribute :name, String + attribute :value, String + attribute :public, Boolean + attribute :repository_id, Integer + + def repository + @repository ||= Models::Repository.find(repository_id) + end + + validates_each :id, :name do |record, attr, value| + others = record.repository.env_vars.select { |ev| ev.id != record.id } + record.errors.add(:base, :duplicate_resource) if others.find { |ev| ev.send(attr) == record.send(attr) } + end + end +end diff --git a/lib/travis/api/v3/models/env_vars.rb b/lib/travis/api/v3/models/env_vars.rb new file mode 100644 index 00000000..1e4b7d08 --- /dev/null +++ b/lib/travis/api/v3/models/env_vars.rb @@ -0,0 +1,11 @@ +module Travis::API::V3 + class Models::EnvVars < Travis::Settings::Collection + model Models::EnvVar + + def repository + @repository ||= Models::Repository.find(additional_attributes[:repository_id]) + end + + undef :to_hash + end +end diff --git a/lib/travis/api/v3/models/repository.rb b/lib/travis/api/v3/models/repository.rb index 1b14bd5f..dfbd1eba 100644 --- a/lib/travis/api/v3/models/repository.rb +++ b/lib/travis/api/v3/models/repository.rb @@ -75,5 +75,11 @@ module Travis::API::V3 def admin_settings @admin_settings ||= Models::AdminSettings.new(settings) end + + def env_vars + @env_vars ||= Models::EnvVars.new.tap do |vars| + vars.load(settings.fetch('env_vars', []), repository_id: self.id) + end + end end end diff --git a/lib/travis/api/v3/queries/env_var.rb b/lib/travis/api/v3/queries/env_var.rb new file mode 100644 index 00000000..4ffaeab9 --- /dev/null +++ b/lib/travis/api/v3/queries/env_var.rb @@ -0,0 +1,21 @@ +module Travis::API::V3 + class Queries::EnvVar < Query + params :id, :name, :value, :public, prefix: :env_var + + def find(repository) + repository.env_vars.find(id) + end + + def update(repository) + if env_var = find(repository) + env_var.update(env_var_params) + repository.save! + env_var + end + end + + def delete(repository) + repository.env_vars.destroy(id) + end + end +end diff --git a/lib/travis/api/v3/queries/env_vars.rb b/lib/travis/api/v3/queries/env_vars.rb new file mode 100644 index 00000000..0544cbbe --- /dev/null +++ b/lib/travis/api/v3/queries/env_vars.rb @@ -0,0 +1,27 @@ +module Travis::API::V3 + class Queries::EnvVars < Query + params :id, :name, :value, :public, prefix: :env_var + + def find(repository) + repository.env_vars + end + + def create(repository) + env_var = repository.env_vars.create(env_var_params) + handle_errors(env_var) unless env_var.valid? + repository.save! + env_var + end + + private + + def handle_errors(env_var) + base = env_var.errors[:base] + case + when base.include?(:format) then raise WrongParams + when base.include?(:duplicate_resource) then raise DuplicateResource + else raise ServerError + end + end + end +end diff --git a/lib/travis/api/v3/renderer/env_var.rb b/lib/travis/api/v3/renderer/env_var.rb new file mode 100644 index 00000000..6d2f955a --- /dev/null +++ b/lib/travis/api/v3/renderer/env_var.rb @@ -0,0 +1,5 @@ +module Travis::API::V3 + class Renderer::EnvVar < Renderer::ModelRenderer + representation :standard, :id, :name, :value, :public + end +end diff --git a/lib/travis/api/v3/renderer/env_vars.rb b/lib/travis/api/v3/renderer/env_vars.rb new file mode 100644 index 00000000..9a47a84c --- /dev/null +++ b/lib/travis/api/v3/renderer/env_vars.rb @@ -0,0 +1,6 @@ +module Travis::API::V3 + class Renderer::EnvVars < Renderer::CollectionRenderer + type :env_vars + collection_key :env_vars + end +end diff --git a/lib/travis/api/v3/routes.rb b/lib/travis/api/v3/routes.rb index 585dc1d8..7767527e 100644 --- a/lib/travis/api/v3/routes.rb +++ b/lib/travis/api/v3/routes.rb @@ -123,6 +123,19 @@ module Travis::API::V3 get :find patch :update end + + resource :env_vars do + route '/env_vars' + get :for_repository + post :create + end + + resource :env_var do + route '/env_var/{env_var.id}' + get :find + patch :update + delete :delete + end end resource :user do diff --git a/lib/travis/api/v3/services.rb b/lib/travis/api/v3/services.rb index 32b30b7c..8a48c895 100644 --- a/lib/travis/api/v3/services.rb +++ b/lib/travis/api/v3/services.rb @@ -11,6 +11,8 @@ module Travis::API::V3 Builds = Module.new { extend Services } Cron = Module.new { extend Services } Crons = Module.new { extend Services } + EnvVar = Module.new { extend Services } + EnvVars = Module.new { extend Services } Job = Module.new { extend Services } Jobs = Module.new { extend Services } Lint = Module.new { extend Services } diff --git a/lib/travis/api/v3/services/env_var/delete.rb b/lib/travis/api/v3/services/env_var/delete.rb new file mode 100644 index 00000000..d11f20f0 --- /dev/null +++ b/lib/travis/api/v3/services/env_var/delete.rb @@ -0,0 +1,12 @@ +module Travis::API::V3 + class Services::EnvVar::Delete < Service + params :id, prefix: :repository + params :id, prefix: :env_var + + def run! + raise LoginRequired unless access_control.logged_in? or access_control.full_access? + raise NotFound unless repository = find(:repository) + query.delete(repository) + end + end +end diff --git a/lib/travis/api/v3/services/env_var/find.rb b/lib/travis/api/v3/services/env_var/find.rb new file mode 100644 index 00000000..476c1b3a --- /dev/null +++ b/lib/travis/api/v3/services/env_var/find.rb @@ -0,0 +1,12 @@ +module Travis::API::V3 + class Services::EnvVar::Find < Service + params :id, prefix: :repository + params :id, prefix: :env_var + + def run! + raise LoginRequired unless access_control.logged_in? or access_control.full_access? + raise NotFound unless repository = find(:repository) + query.find(repository) + end + end +end diff --git a/lib/travis/api/v3/services/env_var/update.rb b/lib/travis/api/v3/services/env_var/update.rb new file mode 100644 index 00000000..6caa751a --- /dev/null +++ b/lib/travis/api/v3/services/env_var/update.rb @@ -0,0 +1,12 @@ +module Travis::API::V3 + class Services::EnvVar::Update < Service + params :id, prefix: :repository + params :id, :name, :value, :public, prefix: :env_var + + def run! + raise LoginRequired unless access_control.logged_in? or access_control.full_access? + raise NotFound unless repository = find(:repository) + query.update(repository) + end + end +end diff --git a/lib/travis/api/v3/services/env_vars/create.rb b/lib/travis/api/v3/services/env_vars/create.rb new file mode 100644 index 00000000..02839fc2 --- /dev/null +++ b/lib/travis/api/v3/services/env_vars/create.rb @@ -0,0 +1,13 @@ +module Travis::API::V3 + class Services::EnvVars::Create < Service + params :id, prefix: :repository + params :id, :name, :value, :public, prefix: :env_var + + def run! + raise LoginRequired unless access_control.logged_in? or access_control.full_access? + raise NotFound unless repository = find(:repository) + env_var = query(:env_vars).create(repository) + result(:env_var, env_var, status: 201) + end + end +end diff --git a/lib/travis/api/v3/services/env_vars/for_repository.rb b/lib/travis/api/v3/services/env_vars/for_repository.rb new file mode 100644 index 00000000..1cb1b4cf --- /dev/null +++ b/lib/travis/api/v3/services/env_vars/for_repository.rb @@ -0,0 +1,9 @@ +module Travis::API::V3 + class Services::EnvVars::ForRepository < Service + def run! + raise LoginRequired unless access_control.logged_in? or access_control.full_access? + raise NotFound unless repository = find(:repository) + find(:env_vars, repository) + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 748561f5..3d6a4f3a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -23,6 +23,40 @@ require 'support/payloads' require 'support/private_key' require 'support/s3' require 'support/test_helpers' +require 'support/shared_examples' + +Travis.logger = Logger.new(StringIO.new) +Travis::Api::App.setup +Travis.config.client_domain = "www.example.com" +Travis.config.endpoints.ssh_key = true + +module TestHelpers + include Sinatra::TestHelpers + + def custom_endpoints + @custom_endpoints ||= [] + end + + def add_settings_endpoint(name, options = {}) + if options[:singleton] + Travis::Api::App::SingletonSettingsEndpoint.subclass(name) + else + Travis::Api::App::SettingsEndpoint.subclass(name) + end + set_app Travis::Api::App.new + end + + def add_endpoint(prefix, &block) + endpoint = Sinatra.new(Travis::Api::App::Endpoint, &block) + endpoint.set(prefix: prefix) + set_app Travis::Api::App.new + custom_endpoints << endpoint + end + + def parsed_body + MultiJson.decode(body) + end +end RSpec.configure do |c| c.mock_framework = :mocha diff --git a/spec/support/shared_examples.rb b/spec/support/shared_examples.rb new file mode 100644 index 00000000..b1964bfb --- /dev/null +++ b/spec/support/shared_examples.rb @@ -0,0 +1,34 @@ +RSpec.shared_examples 'not authenticated' do + example { expect(last_response.status).to eq(403) } + example do + expect(JSON.load(body)).to eq( + '@type' => 'error', + 'error_type' => 'login_required', + 'error_message' => 'login required' + ) + end +end + +RSpec.shared_examples 'missing repo' do + example { expect(last_response.status).to eq(404) } + example do + expect(JSON.load(body)).to eq( + '@type' => 'error', + 'error_message' => 'repository not found (or insufficient access)', + 'error_type' => 'not_found', + 'resource_type' => 'repository' + ) + end +end + +RSpec.shared_examples 'missing env_var' do + example { expect(last_response.status).to eq 404 } + example do + expect(JSON.load(body)).to eq( + '@type' => 'error', + 'error_message' => 'env_var not found (or insufficient access)', + 'error_type' => 'not_found', + 'resource_type' => 'env_var' + ) + end +end diff --git a/spec/v3/services/env_var/delete_spec.rb b/spec/v3/services/env_var/delete_spec.rb new file mode 100644 index 00000000..9f8879c8 --- /dev/null +++ b/spec/v3/services/env_var/delete_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Travis::API::V3::Services::EnvVar::Delete do + let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } + let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } + let(:env_var) { { id: 'abc', name: 'FOO', value: 'bar', public: true, repository_id: repo.id } } + let(:auth_headers) { { 'HTTP_AUTHORIZATION' => "token #{token}" } } + + describe 'not authenticated' do + before { delete("/v3/repo/#{repo.id}/env_var/#{env_var[:id]}") } + include_examples 'not authenticated' + end + + describe 'authenticated, missing repo' do + before { delete("/v3/repo/999999999/env_var/foo", {}, auth_headers) } + include_examples 'missing repo' + end + + describe 'authenticated, missing repo, missing env var' do + before { delete("/v3/repo/#{repo.id}/env_var/#{env_var[:id]}", {}, auth_headers) } + include_examples 'missing env_var' + end + + describe 'authenticated, missing repo, existing env var' do + before do + repo.update_attributes(settings: JSON.generate(env_vars: [env_var])) + delete("/v3/repo/#{repo.id}/env_var/#{env_var[:id]}", {}, auth_headers) + end + + example { expect(last_response.status).to eq 200 } + example { pending 'should we return an empty body here?' } + end +end diff --git a/spec/v3/services/env_var/find_spec.rb b/spec/v3/services/env_var/find_spec.rb new file mode 100644 index 00000000..0b9081b7 --- /dev/null +++ b/spec/v3/services/env_var/find_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +describe Travis::API::V3::Services::EnvVar::Find do + let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } + let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } + let(:env_var) { { id: 'abc', name: 'FOO', value: 'bar', public: true, repository_id: repo.id } } + let(:auth_headers) { { 'HTTP_AUTHORIZATION' => "token #{token}" } } + + describe 'not authenticated' do + before { get("/v3/repo/#{repo.id}/env_var/#{env_var[:id]}") } + include_examples 'not authenticated' + end + + describe 'authenticated, missing repo' do + before { get("/v3/repo/999999999/env_var/foo", {}, auth_headers) } + include_examples 'missing repo' + end + + describe 'authenticated, existing repo, missing env var' do + before { get("/v3/repo/#{repo.id}/env_var/#{env_var[:id]}", {}, auth_headers) } + include_examples 'missing env_var' + end + + describe 'authenticated, existing repo, existing env var' do + before do + repo.update_attributes(settings: JSON.generate(env_vars: [env_var])) + get("/v3/repo/#{repo.id}/env_var/#{env_var[:id]}", {}, auth_headers) + end + + example { expect(last_response.status).to eq 200 } + example do + expect(JSON.load(body)).to eq( + '@type' => 'env_var', + '@href' => "/v3/repo/#{repo.id}/env_var/#{env_var[:id]}", + '@representation' => 'standard', + 'id' => env_var[:id], + 'name' => env_var[:name], + 'public' => env_var[:public], + 'value' => env_var[:value] + ) + end + end +end diff --git a/spec/v3/services/env_var/update_spec.rb b/spec/v3/services/env_var/update_spec.rb new file mode 100644 index 00000000..e23359c7 --- /dev/null +++ b/spec/v3/services/env_var/update_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe Travis::API::V3::Services::EnvVar::Update do + let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } + let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } + let(:env_var) { { id: 'abc', name: 'FOO', value: 'bar', public: true, repository_id: repo.id } } + let(:auth_headers) { { 'HTTP_AUTHORIZATION' => "token #{token}" } } + let(:json_headers) { { 'CONTENT_TYPE' => 'application/json' } } + + describe 'not authenticated' do + before { patch("/v3/repo/#{repo.id}/env_var/#{env_var[:id]}") } + include_examples 'not authenticated' + end + + describe 'authenticated, missing repo' do + before { patch("/v3/repo/999999999/env_var/#{env_var[:id]}", {}, auth_headers) } + include_examples 'missing repo' + end + + describe 'authenticated, existing repo, missing env var' do + before { patch("/v3/repo/#{repo.id}/env_var/foo", {}, auth_headers) } + include_examples 'missing env_var' + end + + describe 'authenticated, existing repo, existing env var' do + let(:params) do + { + 'env_var.name' => 'QUX' + } + end + + before do + repo.update_attributes(settings: JSON.generate(env_vars: [env_var])) + patch("/v3/repo/#{repo.id}/env_var/#{env_var[:id]}", JSON.generate(params), auth_headers.merge(json_headers)) + end + + example { expect(last_response.status).to eq 200 } + example do + expect(JSON.load(body)).to eq( + '@type' => 'env_var', + '@href' => '/v3/repo/1/env_var/abc', + '@representation' => 'standard', + 'id' => env_var[:id], + 'name' => params['env_var.name'], + 'value' => env_var[:value], + 'public' => env_var[:public] + ) + end + end +end diff --git a/spec/v3/services/env_vars/create_spec.rb b/spec/v3/services/env_vars/create_spec.rb new file mode 100644 index 00000000..52230df1 --- /dev/null +++ b/spec/v3/services/env_vars/create_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe Travis::API::V3::Services::EnvVars::Create do + let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } + let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } + let(:auth_headers) { { 'HTTP_AUTHORIZATION' => "token #{token}" } } + let(:json_headers) { { 'CONTENT_TYPE' => 'application/json' } } + + describe 'not authenticated' do + before { post("/v3/repo/#{repo.id}/env_vars", {}) } + include_examples 'not authenticated' + end + + describe 'authenticated, repo missing' do + before { post("/v3/repo/99999999/env_vars", {}, auth_headers) } + include_examples 'missing repo' + end + + describe 'authenticated, existing repo, env var already exists' do + let(:params) do + { + 'env_var.name' => 'FOO', + 'env_var.value' => 'bar', + 'env_var.public' => false + } + end + + before do + repo.update_attributes(settings: JSON.generate(env_vars: [{ id: 'abc', name: 'FOO', value: 'bar', public: false }])) + post("/v3/repo/#{repo.id}/env_vars", JSON.generate(params), auth_headers.merge(json_headers)) + end + + example { expect(last_response.status).to eq 409 } + example do + expect(JSON.load(body)).to eq( + '@type' => 'error', + 'error_message' => 'resource already exists', + 'error_type' => 'duplicate_resource' + ) + end + end + + describe 'authenticated, existing repo, env var is new' do + let(:params) do + { + 'env_var.name' => 'FOO', + 'env_var.value' => 'bar', + 'env_var.public' => false + } + end + + before { post("/v3/repo/#{repo.id}/env_vars", JSON.generate(params), auth_headers.merge(json_headers)) } + + example { expect(last_response.status).to eq 201 } + example do + response = JSON.load(body) + expect(response).to include( + '@type' => 'env_var', + '@representation' => 'standard', + 'name' => 'FOO', + 'value' => 'bar', + 'public' => false + ) + expect(response).to include('@href', 'id') + end + end +end diff --git a/spec/v3/services/env_vars/for_repository_spec.rb b/spec/v3/services/env_vars/for_repository_spec.rb new file mode 100644 index 00000000..60ade927 --- /dev/null +++ b/spec/v3/services/env_vars/for_repository_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Travis::API::V3::Services::EnvVars::ForRepository do + let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } + let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } + let(:env_var) { { id: 'abc', name: 'FOO', value: 'bar', public: true, repository_id: repo.id } } + let(:auth_headers) { { 'HTTP_AUTHORIZATION' => "token #{token}" } } + + describe 'not authenticated' do + before { get("/v3/repo/#{repo.id}/env_vars") } + include_examples 'not authenticated' + end + + describe 'authenticated, missing repo' do + before { get("/v3/repo/999999999/env_vars", {}, auth_headers) } + include_examples 'missing repo' + end + + describe 'authenticated, existing repo, no env vars' do + before do + get("/v3/repo/#{repo.id}/env_vars", {}, auth_headers) + end + example { expect(last_response.status).to eq(200) } + example do + expect(JSON.load(body)).to eq( + '@type' => 'env_vars', + '@href' => "/v3/repo/#{repo.id}/env_vars", + '@representation' => 'standard', + 'env_vars' => [] + ) + end + end + + describe 'authenticated, existing repo, existing env vars' do + before do + repo.update_attributes(settings: JSON.generate(env_vars: [env_var])) + get("/v3/repo/#{repo.id}/env_vars", {}, auth_headers) + end + example { expect(last_response.status).to eq(200) } + example do + expect(JSON.load(body)).to eq( + '@type' => 'env_vars', + '@href' => "/v3/repo/#{repo.id}/env_vars", + '@representation' => 'standard', + 'env_vars' => [ + { + '@type' => 'env_var', + '@href' => "/v3/repo/#{repo.id}/env_var/#{env_var[:id]}", + '@representation' => 'standard', + 'id' => env_var[:id], + 'name' => env_var[:name], + 'value' => env_var[:value], + 'public' => env_var[:public] + } + ] + ) + end + end +end diff --git a/spec/v3/services/settings_spec.rb b/spec/v3/services/settings_spec.rb index b8189275..11b839b5 100644 --- a/spec/v3/services/settings_spec.rb +++ b/spec/v3/services/settings_spec.rb @@ -7,14 +7,7 @@ describe Travis::API::V3::Services::Settings, set_app: true do describe :Find do describe 'not authenticated' do before { get("/v3/repo/#{repo.id}/settings") } - example { expect(last_response.status).to eq(403) } - example do - expect(JSON.load(body)).to eq( - '@type' => 'error', - 'error_type' => 'login_required', - 'error_message' => 'login required' - ) - end + include_examples 'not authenticated' end describe 'authenticated, missing repo' do From 00fb01bd7a722873d78d1dceda6b8d7f9888a9fc Mon Sep 17 00:00:00 2001 From: Joe Corcoran Date: Tue, 21 Jun 2016 12:23:34 +0200 Subject: [PATCH 3/7] Remove some duplication --- lib/travis/api/v3/access_control/generic.rb | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/travis/api/v3/access_control/generic.rb b/lib/travis/api/v3/access_control/generic.rb index f3671da7..30be8902 100644 --- a/lib/travis/api/v3/access_control/generic.rb +++ b/lib/travis/api/v3/access_control/generic.rb @@ -92,22 +92,15 @@ module Travis::API::V3 private_repository_visible?(repository) end - def settings_visible?(settings) - repository_visible?(settings.repository) - end - - def env_vars_visible?(env_vars) - repository_visible?(env_vars.repository) - end - - def env_var_visible?(env_var) - repository_visible?(env_var.repository) - end - def private_repository_visible?(repository) false end + def repository_attr_visible?(record) + repository_visible?(record.repository) + end + [:settings_visible?, :env_vars_visible?, :env_var_visible?].each { |m| alias_method m, :repository_attr_visible? } + def public_api? !Travis.config.private_api end From 81e93ca710cd72167b95af29899fbacbe7803a1f Mon Sep 17 00:00:00 2001 From: Joe Corcoran Date: Tue, 21 Jun 2016 12:31:42 +0200 Subject: [PATCH 4/7] Better style in error handling --- lib/travis/api/v3/queries/env_vars.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/travis/api/v3/queries/env_vars.rb b/lib/travis/api/v3/queries/env_vars.rb index 0544cbbe..9c92d4d6 100644 --- a/lib/travis/api/v3/queries/env_vars.rb +++ b/lib/travis/api/v3/queries/env_vars.rb @@ -17,11 +17,9 @@ module Travis::API::V3 def handle_errors(env_var) base = env_var.errors[:base] - case - when base.include?(:format) then raise WrongParams - when base.include?(:duplicate_resource) then raise DuplicateResource - else raise ServerError - end + raise WrongParams if base.include?(:format) + raise DuplicateResource if base.include?(:duplicate_resource) + raise ServerError end end end From 88d900004232953f43039e0ac9584b6df5a0e147 Mon Sep 17 00:00:00 2001 From: Joe Corcoran Date: Tue, 21 Jun 2016 13:17:44 +0200 Subject: [PATCH 5/7] Refactor repeated pattern into extracted method Seems like we'll be checking login status before finding a resource a lot, so here's a standard way to do it. --- lib/travis/api/v3/access_control/generic.rb | 4 ++++ lib/travis/api/v3/service.rb | 5 +++++ lib/travis/api/v3/services/env_var/delete.rb | 3 +-- lib/travis/api/v3/services/env_var/find.rb | 3 +-- lib/travis/api/v3/services/env_var/update.rb | 3 +-- lib/travis/api/v3/services/env_vars/create.rb | 3 +-- lib/travis/api/v3/services/env_vars/for_repository.rb | 3 +-- 7 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/travis/api/v3/access_control/generic.rb b/lib/travis/api/v3/access_control/generic.rb index 30be8902..48b4958f 100644 --- a/lib/travis/api/v3/access_control/generic.rb +++ b/lib/travis/api/v3/access_control/generic.rb @@ -30,6 +30,10 @@ module Travis::API::V3 false end + def full_access_or_logged_in? + full_access? || logged_in? + end + def visible_repositories(list) # naïve implementation, replaced with smart implementation in specific subclasses return list if full_access? diff --git a/lib/travis/api/v3/service.rb b/lib/travis/api/v3/service.rb index fa63ecf1..13b8f2aa 100644 --- a/lib/travis/api/v3/service.rb +++ b/lib/travis/api/v3/service.rb @@ -66,6 +66,11 @@ module Travis::API::V3 object end + def check_login_and_find(*args) + raise LoginRequired unless access_control.full_access_or_logged_in? + find(*args) + end + def not_found(actually_not_found = false, type = nil) type, actually_not_found = actually_not_found, false if actually_not_found.is_a? Symbol error = actually_not_found ? EntityMissing : NotFound diff --git a/lib/travis/api/v3/services/env_var/delete.rb b/lib/travis/api/v3/services/env_var/delete.rb index d11f20f0..3ea473ab 100644 --- a/lib/travis/api/v3/services/env_var/delete.rb +++ b/lib/travis/api/v3/services/env_var/delete.rb @@ -4,8 +4,7 @@ module Travis::API::V3 params :id, prefix: :env_var def run! - raise LoginRequired unless access_control.logged_in? or access_control.full_access? - raise NotFound unless repository = find(:repository) + repository = check_login_and_find(:repository) query.delete(repository) end end diff --git a/lib/travis/api/v3/services/env_var/find.rb b/lib/travis/api/v3/services/env_var/find.rb index 476c1b3a..17258043 100644 --- a/lib/travis/api/v3/services/env_var/find.rb +++ b/lib/travis/api/v3/services/env_var/find.rb @@ -4,8 +4,7 @@ module Travis::API::V3 params :id, prefix: :env_var def run! - raise LoginRequired unless access_control.logged_in? or access_control.full_access? - raise NotFound unless repository = find(:repository) + repository = check_login_and_find(:repository) query.find(repository) end end diff --git a/lib/travis/api/v3/services/env_var/update.rb b/lib/travis/api/v3/services/env_var/update.rb index 6caa751a..e83e9256 100644 --- a/lib/travis/api/v3/services/env_var/update.rb +++ b/lib/travis/api/v3/services/env_var/update.rb @@ -4,8 +4,7 @@ module Travis::API::V3 params :id, :name, :value, :public, prefix: :env_var def run! - raise LoginRequired unless access_control.logged_in? or access_control.full_access? - raise NotFound unless repository = find(:repository) + repository = check_login_and_find(:repository) query.update(repository) end end diff --git a/lib/travis/api/v3/services/env_vars/create.rb b/lib/travis/api/v3/services/env_vars/create.rb index 02839fc2..a2fd3ed1 100644 --- a/lib/travis/api/v3/services/env_vars/create.rb +++ b/lib/travis/api/v3/services/env_vars/create.rb @@ -4,8 +4,7 @@ module Travis::API::V3 params :id, :name, :value, :public, prefix: :env_var def run! - raise LoginRequired unless access_control.logged_in? or access_control.full_access? - raise NotFound unless repository = find(:repository) + repository = check_login_and_find(:repository) env_var = query(:env_vars).create(repository) result(:env_var, env_var, status: 201) end diff --git a/lib/travis/api/v3/services/env_vars/for_repository.rb b/lib/travis/api/v3/services/env_vars/for_repository.rb index 1cb1b4cf..60a88577 100644 --- a/lib/travis/api/v3/services/env_vars/for_repository.rb +++ b/lib/travis/api/v3/services/env_vars/for_repository.rb @@ -1,8 +1,7 @@ module Travis::API::V3 class Services::EnvVars::ForRepository < Service def run! - raise LoginRequired unless access_control.logged_in? or access_control.full_access? - raise NotFound unless repository = find(:repository) + repository = check_login_and_find(:repository) find(:env_vars, repository) end end From 769ae71076874b38fb5a5d92e3143a985387fb87 Mon Sep 17 00:00:00 2001 From: Joe Corcoran Date: Wed, 22 Jun 2016 16:50:11 +0200 Subject: [PATCH 6/7] Fix service specs --- spec/v3/services/env_var/delete_spec.rb | 2 +- spec/v3/services/env_var/find_spec.rb | 2 +- spec/v3/services/env_var/update_spec.rb | 2 +- spec/v3/services/env_vars/create_spec.rb | 2 +- spec/v3/services/env_vars/for_repository_spec.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/v3/services/env_var/delete_spec.rb b/spec/v3/services/env_var/delete_spec.rb index 9f8879c8..870c69e9 100644 --- a/spec/v3/services/env_var/delete_spec.rb +++ b/spec/v3/services/env_var/delete_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Travis::API::V3::Services::EnvVar::Delete do +describe Travis::API::V3::Services::EnvVar::Delete, set_app: true do let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } let(:env_var) { { id: 'abc', name: 'FOO', value: 'bar', public: true, repository_id: repo.id } } diff --git a/spec/v3/services/env_var/find_spec.rb b/spec/v3/services/env_var/find_spec.rb index 0b9081b7..f80cfdc1 100644 --- a/spec/v3/services/env_var/find_spec.rb +++ b/spec/v3/services/env_var/find_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Travis::API::V3::Services::EnvVar::Find do +describe Travis::API::V3::Services::EnvVar::Find, set_app: true do let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } let(:env_var) { { id: 'abc', name: 'FOO', value: 'bar', public: true, repository_id: repo.id } } diff --git a/spec/v3/services/env_var/update_spec.rb b/spec/v3/services/env_var/update_spec.rb index e23359c7..bac9a065 100644 --- a/spec/v3/services/env_var/update_spec.rb +++ b/spec/v3/services/env_var/update_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Travis::API::V3::Services::EnvVar::Update do +describe Travis::API::V3::Services::EnvVar::Update, set_app: true do let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } let(:env_var) { { id: 'abc', name: 'FOO', value: 'bar', public: true, repository_id: repo.id } } diff --git a/spec/v3/services/env_vars/create_spec.rb b/spec/v3/services/env_vars/create_spec.rb index 52230df1..1755e593 100644 --- a/spec/v3/services/env_vars/create_spec.rb +++ b/spec/v3/services/env_vars/create_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Travis::API::V3::Services::EnvVars::Create do +describe Travis::API::V3::Services::EnvVars::Create, set_app: true do let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } let(:auth_headers) { { 'HTTP_AUTHORIZATION' => "token #{token}" } } diff --git a/spec/v3/services/env_vars/for_repository_spec.rb b/spec/v3/services/env_vars/for_repository_spec.rb index 60ade927..fd20de9d 100644 --- a/spec/v3/services/env_vars/for_repository_spec.rb +++ b/spec/v3/services/env_vars/for_repository_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Travis::API::V3::Services::EnvVars::ForRepository do +describe Travis::API::V3::Services::EnvVars::ForRepository, set_app: true do let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first_or_create } let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } let(:env_var) { { id: 'abc', name: 'FOO', value: 'bar', public: true, repository_id: repo.id } } From 0aeec3593513cd05e82cc58bcaa3c3b8d353d38c Mon Sep 17 00:00:00 2001 From: Joe Corcoran Date: Wed, 22 Jun 2016 16:59:49 +0200 Subject: [PATCH 7/7] Fix duplicate lines from rebase --- spec/spec_helper.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3d6a4f3a..1023fb79 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,11 +25,6 @@ require 'support/s3' require 'support/test_helpers' require 'support/shared_examples' -Travis.logger = Logger.new(StringIO.new) -Travis::Api::App.setup -Travis.config.client_domain = "www.example.com" -Travis.config.endpoints.ssh_key = true - module TestHelpers include Sinatra::TestHelpers