From 595163619d3cc9c74baf8ab9e9cc425a5647dfe7 Mon Sep 17 00:00:00 2001 From: Konstantin Haase Date: Tue, 28 Apr 2015 14:58:21 +0200 Subject: [PATCH] API v3: allow eager loading owner repos --- lib/travis/api/v3/renderer.rb | 11 ++-- lib/travis/api/v3/renderer/model_renderer.rb | 21 +++++--- lib/travis/api/v3/renderer/owner.rb | 16 +++++- lib/travis/api/v3/result.rb | 8 +-- lib/travis/api/v3/router.rb | 2 +- lib/travis/api/v3/service.rb | 8 ++- spec/v3/result_spec.rb | 10 ++-- spec/v3/services/owner/find_spec.rb | 54 +++++++++++++++++++- 8 files changed, 102 insertions(+), 28 deletions(-) diff --git a/lib/travis/api/v3/renderer.rb b/lib/travis/api/v3/renderer.rb index a2c2b1c5..c1518399 100644 --- a/lib/travis/api/v3/renderer.rb +++ b/lib/travis/api/v3/renderer.rb @@ -42,11 +42,12 @@ module Travis::API::V3 def render_value(value, **options) case value - when Hash then value.map { |k, v| [k, render_value(v)] }.to_h - when Array then value.map { |v | render_value(v) } - when *PRIMITIVE then value - when Time then value.strftime('%Y-%m-%dT%H:%M:%SZ') - when Model then render_model(value, **options) + when Hash then value.map { |k, v| [k, render_value(v)] }.to_h + when Array then value.map { |v | render_value(v) } + when *PRIMITIVE then value + when Time then value.strftime('%Y-%m-%dT%H:%M:%SZ') + when Model then render_model(value, **options) + when ActiveRecord::Relation then render_value(value.to_a, **options) else raise ArgumentError, 'cannot render %p (%p)' % [value.class, value] end end diff --git a/lib/travis/api/v3/renderer/model_renderer.rb b/lib/travis/api/v3/renderer/model_renderer.rb index 07944fec..2f65e201 100644 --- a/lib/travis/api/v3/renderer/model_renderer.rb +++ b/lib/travis/api/v3/renderer/model_renderer.rb @@ -34,15 +34,16 @@ module Travis::API::V3 new(model, **options).render(representation) end - attr_reader :model, :options, :script_name, :include, :included + attr_reader :model, :options, :script_name, :include, :included, :access_control attr_writer :href - def initialize(model, script_name: nil, include: [], included: [], **options) - @model = model - @options = options - @script_name = script_name - @include = include - @included = included + def initialize(model, script_name: nil, include: [], included: [], access_control: nil, **options) + @model = model + @options = options + @script_name = script_name + @include = include + @included = included + @access_control = access_control end def href @@ -69,7 +70,11 @@ module Travis::API::V3 nested_included = included + [model] modes = {} - excepted_type = result[:@type].to_s if include.any? + if include.any? + excepted_type = result[:@type].to_s + fields = fields.dup + end + include.each do |qualified_field| raise WrongParams, 'illegal format for include parameter'.freeze unless /\A(?\w+)\.(?\w+)\Z$/ =~ qualified_field next if prefix != excepted_type diff --git a/lib/travis/api/v3/renderer/owner.rb b/lib/travis/api/v3/renderer/owner.rb index 1d694b12..ead5edc0 100644 --- a/lib/travis/api/v3/renderer/owner.rb +++ b/lib/travis/api/v3/renderer/owner.rb @@ -5,7 +5,19 @@ module Travis::API::V3 class Renderer::Owner < Renderer::ModelRenderer include Renderer::AvatarURL - representation(:minimal, :id, :login) - representation(:standard, :id, :login, :name, :github_id, :avatar_url) + representation(:minimal, :id, :login) + representation(:standard, :id, :login, :name, :github_id, :avatar_url) + representation(:additional, :repositories) + + def initialize(*) + super + + owner_includes = include.select { |i| i.start_with?('owner.'.freeze) } + owner_includes.each { |i| include << i.sub('owner.'.freeze, "#{self.class.type}.") } + end + + def repositories + access_control.visible_repositories(@model.repositories) + end end end diff --git a/lib/travis/api/v3/result.rb b/lib/travis/api/v3/result.rb index cb7a8efb..1cf5733b 100644 --- a/lib/travis/api/v3/result.rb +++ b/lib/travis/api/v3/result.rb @@ -1,9 +1,9 @@ module Travis::API::V3 class Result - attr_accessor :type, :resource, :status, :href + attr_accessor :access_control, :type, :resource, :status, :href - def initialize(type, resource = [], status: 200) - @type, @resource, @status = type, resource, status + def initialize(access_control, type, resource = [], status: 200) + @access_control, @type, @resource, @status = access_control, type, resource, status end def respond_to_missing?(method, *) @@ -19,7 +19,7 @@ module Travis::API::V3 href = self.href href = V3.location(env) if href.nil? and env['REQUEST_METHOD'.freeze] == 'GET'.freeze include = params['include'.freeze].to_s.split(?,.freeze) - Renderer[type].render(resource, href: href, script_name: env['SCRIPT_NAME'.freeze], include: include) + Renderer[type].render(resource, href: href, script_name: env['SCRIPT_NAME'.freeze], include: include, access_control: access_control) end def method_missing(method, *args) diff --git a/lib/travis/api/v3/router.rb b/lib/travis/api/v3/router.rb index 956d3db9..74247095 100644 --- a/lib/travis/api/v3/router.rb +++ b/lib/travis/api/v3/router.rb @@ -21,7 +21,7 @@ module Travis::API::V3 result = service.run render(result, env_params, env) rescue Error => error - result = Result.new(:error, error) + result = Result.new(access_control, :error, error) headers = error.status == 404 ? CASCADE : {} V3.response(result.render(env_params, env), headers, status: error.status) end diff --git a/lib/travis/api/v3/service.rb b/lib/travis/api/v3/service.rb index 020a6aaf..c8c90249 100644 --- a/lib/travis/api/v3/service.rb +++ b/lib/travis/api/v3/service.rb @@ -61,9 +61,13 @@ module Travis::API::V3 self.class.result_type end + def result(*args) + Result.new(access_control, *args) + end + def run not_found unless result = run! - result = Result.new(result_type, result) unless result.is_a? Result + result = result(result_type, result) unless result.is_a? Result result end @@ -75,7 +79,7 @@ module Travis::API::V3 def accepted(**payload) payload[:resource_type] ||= result_type - Result.new(:accepted, payload, status: 202) + result(:accepted, payload, status: 202) end def not_implemented diff --git a/spec/v3/result_spec.rb b/spec/v3/result_spec.rb index b66945b3..3dd75885 100644 --- a/spec/v3/result_spec.rb +++ b/spec/v3/result_spec.rb @@ -1,11 +1,13 @@ require 'spec_helper' describe Travis::API::V3::Result do - subject(:result) { described_class.new(:example) } + let(:access_control) { Object.new } + subject(:result) { described_class.new(access_control, :example) } - example { expect(result.type) .to be == :example } - example { expect(result.resource) .to be == [] } - example { expect(result.example) .to be == [] } + example { expect(result.type) .to be == :example } + example { expect(result.resource) .to be == [] } + example { expect(result.example) .to be == [] } + example { expect(result.access_control) .to be == access_control } example do result << 42 diff --git a/spec/v3/services/owner/find_spec.rb b/spec/v3/services/owner/find_spec.rb index 6717047f..df026231 100644 --- a/spec/v3/services/owner/find_spec.rb +++ b/spec/v3/services/owner/find_spec.rb @@ -7,7 +7,7 @@ describe Travis::API::V3::Services::Owner::Find do after { org.delete } describe 'existing org, public api' do - before { get("/v3/owner/example-org") } + before { get("/v3/owner/example-org") } example { expect(last_response).to be_ok } example { expect(JSON.load(body)).to be == { "@type" => "organization", @@ -20,8 +20,58 @@ describe Travis::API::V3::Services::Owner::Find do }} end + describe 'eager loading repositories via organization.repositories' do + let(:repo) { Repository.new(name: 'example-repo', owner_name: 'example-org', owner_id: org.id, owner_type: 'Organization')} + + before { repo.save! } + after { repo.destroy } + + before { get("/v3/owner/example-org?include=organization.repositories,user.repositories") } + example { expect(last_response).to be_ok } + example { expect(JSON.load(body)).to be == { + "@type" => "organization", + "@href" => "/v3/org/#{org.id}", + "id" => org.id, + "login" => "example-org", + "name" => nil, + "github_id" => nil, + "avatar_url" => nil, + "repositories" => [{ + "@type" => "repository", + "@href" => "/repo/#{repo.id}", + "id" => repo.id, + "slug" => "example-org/example-repo" + }] + }} + end + + describe 'eager loading repositories via owner.repositories' do + let(:repo) { Repository.new(name: 'example-repo', owner_name: 'example-org', owner_id: org.id, owner_type: 'Organization')} + + before { repo.save! } + after { repo.destroy } + + before { get("/v3/owner/example-org?include=owner.repositories") } + example { expect(last_response).to be_ok } + example { expect(JSON.load(body)).to be == { + "@type" => "organization", + "@href" => "/v3/org/#{org.id}", + "id" => org.id, + "login" => "example-org", + "name" => nil, + "github_id" => nil, + "avatar_url" => nil, + "repositories" => [{ + "@type" => "repository", + "@href" => "/repo/#{repo.id}", + "id" => repo.id, + "slug" => "example-org/example-repo" + }] + }} + end + describe 'it is not case sensitive' do - before { get("/v3/owner/example-ORG") } + before { get("/v3/owner/example-ORG") } example { expect(last_response).to be_ok } example { expect(JSON.load(body)).to be == { "@type" => "organization",