diff --git a/Gemfile b/Gemfile index 58bc4852..69604289 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,7 @@ gem 'travis-sidekiqs', github: 'travis-ci/travis-sidekiqs', require: nil, ref: ' gem 'sinatra' gem 'sinatra-contrib', require: nil #github: 'sinatra/sinatra-contrib', require: nil +gem 'active_model_serializers' gem 'unicorn' gem 'sentry-raven', github: 'getsentry/raven-ruby' gem 'yard-sinatra', github: 'rkh/yard-sinatra' diff --git a/Gemfile.lock b/Gemfile.lock index 3dfced19..ea2e6e73 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -96,6 +96,8 @@ GEM rack-cache (~> 1.2) rack-test (~> 0.6.1) sprockets (~> 2.2.1) + active_model_serializers (0.8.1) + activemodel (>= 3.0) activemodel (3.2.17) activesupport (= 3.2.17) builder (~> 3.0.0) @@ -279,6 +281,7 @@ PLATFORMS ruby DEPENDENCIES + active_model_serializers bunny (~> 0.8.0) dalli database_cleaner (~> 0.8.0) diff --git a/lib/travis/api/app.rb b/lib/travis/api/app.rb index c15a6146..80fa6e26 100644 --- a/lib/travis/api/app.rb +++ b/lib/travis/api/app.rb @@ -18,6 +18,7 @@ require 'sidekiq' require 'metriks/reporter/logger' require 'travis/support/log_subscriber/active_record_metrics' require 'fileutils' +require 'travis/api/serializer' # Rack class implementing the HTTP API. # Instances respond to #call. @@ -113,7 +114,11 @@ module Travis::Api use Travis::Api::App::Middleware::Metriks use Travis::Api::App::Middleware::Rewrite - Endpoint.subclasses.each { |e| map(e.prefix) { run(e.new) } } + Endpoint.subclasses.each do |e| + next if e == SettingsEndpoint # TODO: add something like abstract? method to check if + # class should be registered + map(e.prefix) { run(e.new) } + end end end diff --git a/lib/travis/api/app/endpoint/repos.rb b/lib/travis/api/app/endpoint/repos.rb index e0d01cbe..9dafd733 100644 --- a/lib/travis/api/app/endpoint/repos.rb +++ b/lib/travis/api/app/endpoint/repos.rb @@ -41,15 +41,6 @@ class Travis::Api::App respond_with service(:find_repo, params.merge(schema: 'cc')) end - get '/:id/settings/ssh_keys' do - settings = service(:find_repo_settings, params).run - if settings - respond_with({ settings: settings.obfuscated }, version: :v2) - else - status 404 - end - end - # Get settings for a given repository # get '/:id/settings' do diff --git a/lib/travis/api/app/endpoint/setting_endpoint.rb b/lib/travis/api/app/endpoint/setting_endpoint.rb new file mode 100644 index 00000000..44bc047f --- /dev/null +++ b/lib/travis/api/app/endpoint/setting_endpoint.rb @@ -0,0 +1,94 @@ +require 'travis/api/app' + +class Travis::Api::App + class SettingsEndpoint < Endpoint + set(:prefix) { "/settings/" << name[/[^:]+$/].underscore } + + class << self + # This method checks if class based on a given name exists or creates + # a new SettingsEndpoint subclass, which will be then used as an endpoint + def subclass(name) + class_name = name.to_s.camelize + if Travis::Api::App.const_defined?(class_name) + Travis::Api::App.const_get(class_name) + else + klass = create_settings_class(name) + Travis::Api::App.const_set(class_name, klass) + klass + end + end + + def create_settings_class(name) + klass = Class.new(self) do + define_method(:name) { name } + get("/", scope: :private) do index end + get("/:id", scope: :private) do show end + post("/", scope: :private) do create end + patch("/:id", scope: :private) do update end + delete("/:id", scope: :private) do destroy end + end + end + end + + # Rails style methods for easy overriding + def index + respond_with(collection, type: name, verson: :v2) + end + + def show + respond_with(record, type: singular_name, verson: :v2) + end + + def update + record.update(params[singular_name]) + if record.valid? + repo_settings.save + respond_with(record, type: singular_name, version: :v2) + else + status 422 + respond_with(record, type: :validation_error, version: :v2) + end + end + + def create + record = collection.create(params[singular_name]) + if record.valid? + repo_settings.save + respond_with(record, type: singular_name, version: :v2) + else + status 422 + respond_with(record, type: :validation_error, version: :v2) + end + end + + def destroy + record = collection.destroy(params[:id]) || record_not_found + repo_settings.save + respond_with(record, type: singular_name, verson: :v2) + end + + def singular_name + name.to_s.singularize + end + + def collection + @collection ||= repo_settings.send(name) + end + + # This method can't be called "settings" because it clashes with + # Sinatra's method + def repo_settings + @settings ||= begin + service(:find_repo_settings, id: params['repository_id'].to_i).run + end || halt(404, error: "Couldn't find repository") + end + + def record + collection.find(params[:id]) || record_not_found + end + + def record_not_found + halt(404, { error: "Could not find a requested setting" }) + end + end +end diff --git a/lib/travis/api/app/middleware/rewrite.rb b/lib/travis/api/app/middleware/rewrite.rb index 893feb34..0c7b2e22 100644 --- a/lib/travis/api/app/middleware/rewrite.rb +++ b/lib/travis/api/app/middleware/rewrite.rb @@ -5,6 +5,7 @@ class Travis::Api::App class Rewrite < Middleware FORMAT = %r(\.(json|xml|png|txt|atom|svg)$) V1_REPO_URL = %r(^(/[^/]+/[^/]+(?:/builds(?:/[\d]+)?|/cc)?)$) + SETTINGS_URL = %r(^/settings) helpers :accept @@ -37,7 +38,9 @@ class Travis::Api::App end def redirect_v1_named_repo_path - force_redirect("/repositories#{$1}.#{env['travis.format']}") if request.path =~ V1_REPO_URL + if request.path =~ V1_REPO_URL && request.path !~ SETTINGS_URL + force_redirect("/repositories#{$1}.#{env['travis.format']}") + end end def force_redirect(path) diff --git a/lib/travis/api/app/responders/json.rb b/lib/travis/api/app/responders/json.rb index 012b2be8..3bc3bb8d 100644 --- a/lib/travis/api/app/responders/json.rb +++ b/lib/travis/api/app/responders/json.rb @@ -27,7 +27,13 @@ class Travis::Api::App end def result - builder ? builder.new(resource, params).data : basic_type_resource + if builder + p = params + p[:root] = options[:type] if options[:type] + builder.new(resource, p).data + else + basic_type_resource + end end def builder diff --git a/lib/travis/api/serializer.rb b/lib/travis/api/serializer.rb new file mode 100644 index 00000000..a34ecdff --- /dev/null +++ b/lib/travis/api/serializer.rb @@ -0,0 +1,23 @@ +require 'active_model_serializers' + +module Travis + module Api + class Serializer < ActiveModel::Serializer + def data + as_json + end + end + + class ArraySerializer < ActiveModel::ArraySerializer + def data + as_json + end + + def initialize(resource, options) + options[:each_serializer] ||= Travis::Api::V2::Http.const_get(options[:root].to_s.singularize.camelize) + super(resource, options) + end + end + end +end + diff --git a/lib/travis/api/v2/http.rb b/lib/travis/api/v2/http.rb index 2746d130..6034ef07 100644 --- a/lib/travis/api/v2/http.rb +++ b/lib/travis/api/v2/http.rb @@ -21,6 +21,7 @@ module Travis require 'travis/api/v2/http/request' require 'travis/api/v2/http/ssl_key' require 'travis/api/v2/http/user' + require 'travis/api/v2/http/validation_error' end end end diff --git a/lib/travis/api/v2/http/error.rb b/lib/travis/api/v2/http/error.rb new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/lib/travis/api/v2/http/error.rb @@ -0,0 +1 @@ + diff --git a/lib/travis/api/v2/http/validation_error.rb b/lib/travis/api/v2/http/validation_error.rb new file mode 100644 index 00000000..dc5cfe33 --- /dev/null +++ b/lib/travis/api/v2/http/validation_error.rb @@ -0,0 +1,39 @@ +module Travis + module Api + module V2 + module Http + class ValidationError + attr_reader :resource + + def initialize(resource, options = {}) + @resource = resource + end + + def data + response = { + message: 'Validation failed' + } + resource.errors.to_hash.each do |name, errors| + response['errors'] ||= [] + errors.each do |error_code| + response['errors'] << { field: name, code: code(error_code) } + end + end + + response + end + + def code(error_code) + case error_code + when :blank + 'missing_field' + else + error_code.to_s + end + end + end + end + end + end +end + diff --git a/spec/integration/settings_endpoint_spec.rb b/spec/integration/settings_endpoint_spec.rb new file mode 100644 index 00000000..f7ae0dc7 --- /dev/null +++ b/spec/integration/settings_endpoint_spec.rb @@ -0,0 +1,178 @@ +require 'spec_helper' + +describe Travis::Api::App::SettingsEndpoint do + let(:repo) { Repository.by_slug('svenfuchs/minimal').first } + let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json' } } + + before do + model_class = Class.new(Repository::Settings::Model) do + field :name + field :secret, encrypted: true + + validates :name, presence: true + validates :secret, presence: true + end + collection_class = Class.new(Repository::Settings::Collection) do + model model_class + end + Repository::Settings.class_eval do + register :items, collection_class + end + serializer_class = Class.new(Travis::Api::Serializer) do + attributes :id, :name + end + Travis::Api::V2::Http.const_set(:Item, serializer_class) + Travis::Api::V2::Http.const_set(:Items, Travis::Api::ArraySerializer) + + add_settings_endpoint :items + end + + after do + Travis::Api::App.send :remove_const, :Items + Travis::Api::V2::Http.send :remove_const, :Items + Travis::Api::V2::Http.send :remove_const, :Item + end + + describe 'with authenticated user' do + let(:user) { User.where(login: 'svenfuchs').first } + let(:token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } + let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json', 'HTTP_AUTHORIZATION' => "token #{token}" } } + + before { user.permissions.create!(:repository_id => repo.id, :admin => true, :push => true) } + + describe 'GET /items/:id' do + it 'returns an item' do + settings = repo.settings + item = settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + response = get '/settings/items/' + item.id, { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['item']['name'].should == 'an item' + json['item']['id'].should == item.id + json['item'].should_not have_key('secret') + end + + it 'returns 404 if item can\'t be found' do + response = get '/settings/items/123', { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['error'].should == "Could not find a requested setting" + end + end + + describe 'GET /items' do + it 'returns items list' do + settings = repo.settings + settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + response = get '/settings/items', { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['items'].should have(1).items + item = json['items'].first + item['name'].should == 'an item' + item['id'].should_not be_nil + item.should_not have_key('secret') + end + end + + describe 'POST /items' do + it 'creates a new item' do + params = { repository_id: repo.id, item: { name: 'foo', secret: 'TEH SECRET' } } + response = post '/settings/items', params, headers + json = JSON.parse(response.body) + json['item']['name'].should == 'foo' + json['item']['id'].should_not be_nil + json['item'].should_not have_key('secret') + + item = repo.reload.settings.items.first + item.id.should_not be_nil + item.name.should == 'foo' + item.secret.decrypt.should == 'TEH SECRET' + end + + it 'returns error message if item is invalid' do + params = { repository_id: repo.id } + response = post '/settings/items', params, headers + response.status.should == 422 + + json = JSON.parse(response.body) + json['message'].should == 'Validation failed' + json['errors'].should == [{ + 'field' => 'name', + 'code' => 'missing_field' + }, { + 'field' => 'secret', + 'code' => 'missing_field' + }] + + repo.reload.settings.items.length.should == 0 + end + end + + describe 'PATCH /items/:id' do + it 'should update an item' do + settings = repo.settings + item = settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + params = { repository_id: repo.id, item: { name: 'a new name', secret: 'a new secret' } } + response = patch '/settings/items/' + item.id, params, headers + json = JSON.parse(response.body) + json['item']['name'].should == 'a new name' + json['item']['id'].should == item.id + json['item'].should_not have_key('secret') + + updated_item = repo.reload.settings.items.find(item.id) + updated_item.id.should == item.id + updated_item.name.should == 'a new name' + updated_item.secret.decrypt.should == 'a new secret' + end + + it 'returns an error message if item is invalid' do + settings = repo.settings + item = settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + params = { repository_id: repo.id, item: { name: '' } } + response = patch '/settings/items/' + item.id, params, headers + response.status.should == 422 + + json = JSON.parse(response.body) + json['message'].should == 'Validation failed' + json['errors'].should == [{ + 'field' => 'name', + 'code' => 'missing_field' + }] + + updated_item = repo.reload.settings.items.find(item.id) + updated_item.id.should == item.id + updated_item.name.should == 'an item' + updated_item.secret.decrypt.should == 'TEH SECRET' + end + end + + describe 'DELETE /items/:id' do + it 'should delete an item' do + settings = repo.settings + item = settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + params = { repository_id: repo.id } + response = delete '/settings/items/' + item.id, params, headers + json = JSON.parse(response.body) + json['item']['name'].should == 'an item' + json['item']['id'].should == item.id + json['item'].should_not have_key('secret') + + repo.reload.settings.items.should have(0).items + end + + it 'returns 404 if item can\'t be found' do + response = delete '/settings/items/123', { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['error'].should == "Could not find a requested setting" + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 43db91da..197cb126 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,6 +26,11 @@ module TestHelpers @custom_endpoints ||= [] end + def add_settings_endpoint(name) + Travis::Api::App::SettingsEndpoint.subclass(name) + set_app Travis::Api::App.new + end + def add_endpoint(prefix, &block) endpoint = Sinatra.new(Travis::Api::App::Endpoint, &block) endpoint.set(prefix: prefix)