From a3a9f1282a153fc63afdfbc2daf1bbfd2f2a2f12 Mon Sep 17 00:00:00 2001 From: Konstantin Haase Date: Mon, 9 Mar 2015 16:55:54 +0100 Subject: [PATCH] v3: add ?include=, detect circular dependencies in renderer --- lib/travis/api/v3/models/build.rb | 13 ++++ lib/travis/api/v3/renderer.rb | 6 +- lib/travis/api/v3/renderer/build.rb | 2 +- lib/travis/api/v3/renderer/collection.rb | 6 +- lib/travis/api/v3/renderer/model_renderer.rb | 65 ++++++++++++++---- lib/travis/api/v3/renderer/repository.rb | 2 + lib/travis/api/v3/renderer/user.rb | 8 +++ lib/travis/api/v3/result.rb | 7 +- spec/v3/services/repository/find_spec.rb | 70 +++++++++++++++++--- 9 files changed, 148 insertions(+), 31 deletions(-) create mode 100644 lib/travis/api/v3/renderer/user.rb diff --git a/lib/travis/api/v3/models/build.rb b/lib/travis/api/v3/models/build.rb index 9b7f766b..219a6913 100644 --- a/lib/travis/api/v3/models/build.rb +++ b/lib/travis/api/v3/models/build.rb @@ -6,5 +6,18 @@ module Travis::API::V3 belongs_to :repository, autosave: true belongs_to :owner, polymorphic: true has_many :jobs, as: :source, order: :id, dependent: :destroy + + has_one :branch, + foreign_key: [:repository_id, :name], + primary_key: [:repository_id, :branch], + class_name: 'Travis::API::V3::Models::Branch'.freeze + + def branch_name + read_attribute(:branch) + end + + def branch_name=(value) + write_attribute(:branch, value) + end end end diff --git a/lib/travis/api/v3/renderer.rb b/lib/travis/api/v3/renderer.rb index 7f724b42..ab21513f 100644 --- a/lib/travis/api/v3/renderer.rb +++ b/lib/travis/api/v3/renderer.rb @@ -18,7 +18,7 @@ module Travis::API::V3 expander = EXPANDER_CACHE[[type, script_name, args.keys]] ||= begin resource = Routes.resources.detect { |r| r.identifier == type } - route = resource.route + route = resource.route if resource route &&= Mustermann.new(script_name, type: :identity) + route if script_name and not script_name.empty? key_mapping = {} args.keys.each do |key| @@ -34,8 +34,8 @@ module Travis::API::V3 expander.call(args) end - def render_model(model, type: model.class.name[/[^:]+$/].to_sym, mode: :minimal, **options) - Renderer[type].render(model, mode, **options) + def render_model(model, type: model.class.name[/[^:]+$/].to_sym, mode: nil, **options) + Renderer[type].render(model, mode || :minimal, **options) end def render_value(value, **options) diff --git a/lib/travis/api/v3/renderer/build.rb b/lib/travis/api/v3/renderer/build.rb index 14a4c2b0..a237a925 100644 --- a/lib/travis/api/v3/renderer/build.rb +++ b/lib/travis/api/v3/renderer/build.rb @@ -3,6 +3,6 @@ require 'travis/api/v3/renderer/model_renderer' module Travis::API::V3 class Renderer::Build < Renderer::ModelRenderer representation(:minimal, :id, :number, :state, :duration, :started_at, :finished_at) - representation(:standard, *representations[:minimal], :repository) + representation(:standard, *representations[:minimal], :repository, :branch) end end diff --git a/lib/travis/api/v3/renderer/collection.rb b/lib/travis/api/v3/renderer/collection.rb index 3d56e441..8b3da3bd 100644 --- a/lib/travis/api/v3/renderer/collection.rb +++ b/lib/travis/api/v3/renderer/collection.rb @@ -2,9 +2,9 @@ module Travis::API::V3 module Renderer::Collection extend self - def render(collection_type, entry_type, entries, href: nil, script_name: nil, **additional) - entries &&= entries.map { |entry| Renderer[entry_type].render(entry, script_name: script_name) } - Renderer.clear(:@type => collection_type, :@href => href).merge(collection_type => entries, **additional) + def render(collection_type, entry_type, entries, href: nil, script_name: nil, include: [], included: [], **) + entries &&= entries.map { |entry| Renderer[entry_type].render(entry, script_name: script_name, include: include, included: included) } + Renderer.clear(:@type => collection_type, :@href => href).merge(collection_type => entries) end end end diff --git a/lib/travis/api/v3/renderer/model_renderer.rb b/lib/travis/api/v3/renderer/model_renderer.rb index 2cbc064d..ebe3dac9 100644 --- a/lib/travis/api/v3/renderer/model_renderer.rb +++ b/lib/travis/api/v3/renderer/model_renderer.rb @@ -1,13 +1,21 @@ +require 'set' + module Travis::API::V3 class Renderer::ModelRenderer - def self.type(type = nil) - @type = type if type - @type = name[/[^:]+$/].underscore.to_sym unless defined? @type # allows setting type to nil + REDUNDANT = Object.new + private_constant :REDUNDANT + + def self.type(type = false) + @type = type if type != false + @type = name[/[^:]+$/].underscore.to_sym unless defined? @type # allows setting type to nil @type end def self.representation(name, *fields) - fields.each { |field| class_eval "def #{field}; @model.#{field}; end" unless method_defined?(field) } + fields.each do |field| + class_eval "def #{field}; @model.#{field}; end" unless method_defined?(field) + available_fields << field.to_s + end representations[name] = fields end @@ -15,17 +23,23 @@ module Travis::API::V3 @representations ||= {} end + def self.available_fields + @available_fields ||= Set.new + end + def self.render(model, representation = :standard, **options) new(model, **options).render(representation) end - attr_reader :model, :options, :script_name + attr_reader :model, :options, :script_name, :include, :included attr_writer :href - def initialize(model, script_name: nil, **options) + def initialize(model, script_name: nil, include: [], included: [], **options) @model = model @options = options @script_name = script_name + @include = include + @included = included end def href @@ -34,13 +48,40 @@ module Travis::API::V3 @href = Renderer.href(self.class.type, model.attributes, script_name: script_name) end - def render(representation) - result = {} - result[:@type] = self.class.type if self.class.type - result[:@href] = href if href - fields = self.class.representations.fetch(representation) + def include?(field) + field = "#{self.class.type}.#{field}" if field.is_a? Symbol + include.include?(field) + end + + def render(representation) + if included.include? model + return REDUNDANT unless href + return { :@href => href } + end + + result = {} + result[:@type] = self.class.type if self.class.type + result[:@href] = href if href + fields = self.class.representations.fetch(representation) + nested_included = included + [model] + modes = {} + + excepted_type = result[:@type].to_s if include.any? + 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 + raise WrongParams, 'no field %p to include'.freeze % qualified_field unless self.class.available_fields.include?(field) + + field &&= field.to_sym + fields << field unless fields.include?(field) + modes[field] = :standard + end + + fields.each do |field| + value = Renderer.render_value(send(field), script_name: script_name, include: include, included: nested_included, mode: modes[field]) + result[field] = value unless value == REDUNDANT + end - fields.each { |field| result[field] = Renderer.render_value(send(field), script_name: script_name) } result end end diff --git a/lib/travis/api/v3/renderer/repository.rb b/lib/travis/api/v3/renderer/repository.rb index 947ea727..d483183d 100644 --- a/lib/travis/api/v3/renderer/repository.rb +++ b/lib/travis/api/v3/renderer/repository.rb @@ -10,6 +10,7 @@ module Travis::API::V3 end def owner + return model.owner if include? 'repository.owner'.freeze { :@type => model.owner_type && model.owner_type.downcase, :id => model.owner_id, @@ -19,6 +20,7 @@ module Travis::API::V3 def last_build return nil unless model.last_build_id + return model.last_build if include? 'repository.last_build'.freeze { :@type => 'build'.freeze, :@href => Renderer.href(:build, script_name: script_name, id: model.last_build_id), diff --git a/lib/travis/api/v3/renderer/user.rb b/lib/travis/api/v3/renderer/user.rb new file mode 100644 index 00000000..f6fc870f --- /dev/null +++ b/lib/travis/api/v3/renderer/user.rb @@ -0,0 +1,8 @@ +require 'travis/api/v3/renderer/model_renderer' + +module Travis::API::V3 + class Renderer::User < Renderer::ModelRenderer + representation(:minimal, :id, :login) + representation(:standard, :id, :login, :name, :github_id, :is_syncing, :synced_at) + end +end diff --git a/lib/travis/api/v3/result.rb b/lib/travis/api/v3/result.rb index 022a4ed5..cb7a8efb 100644 --- a/lib/travis/api/v3/result.rb +++ b/lib/travis/api/v3/result.rb @@ -16,9 +16,10 @@ module Travis::API::V3 end def render(params, env) - href = self.href - href = V3.location(env) if href.nil? and env['REQUEST_METHOD'.freeze] == 'GET'.freeze - Renderer[type].render(resource, href: href, script_name: env['SCRIPT_NAME'.freeze]) + 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) end def method_missing(method, *args) diff --git a/spec/v3/services/repository/find_spec.rb b/spec/v3/services/repository/find_spec.rb index b11d334e..034b00a9 100644 --- a/spec/v3/services/repository/find_spec.rb +++ b/spec/v3/services/repository/find_spec.rb @@ -2,11 +2,12 @@ require 'spec_helper' describe Travis::API::V3::Services::Repository::Find do let(:repo) { Repository.by_slug('svenfuchs/minimal').first } + let(:parsed_body) { JSON.load(body) } describe "public repository" do before { get("/v3/repo/#{repo.id}") } example { expect(last_response).to be_ok } - example { expect(JSON.load(body)).to be == { + example { expect(parsed_body).to be == { "@type" => "repository", "@href" => "/v3/repo/#{repo.id}", "id" => repo.id, @@ -48,7 +49,7 @@ describe Travis::API::V3::Services::Repository::Find do describe "missing repository" do before { get("/v3/repo/999999999999999") } example { expect(last_response).to be_not_found } - example { expect(JSON.load(body)).to be == { + example { expect(parsed_body).to be == { "@type" => "error", "error_type" => "not_found", "error_message" => "repository not found (or insufficient access)", @@ -61,7 +62,7 @@ describe Travis::API::V3::Services::Repository::Find do before { get("/v3/repo/#{repo.id}") } after { Travis.config.private_api = false } example { expect(last_response).to be_not_found } - example { expect(JSON.load(body)).to be == { + example { expect(parsed_body).to be == { "@type" => "error", "error_type" => "not_found", "error_message" => "repository not found (or insufficient access)", @@ -74,7 +75,7 @@ describe Travis::API::V3::Services::Repository::Find do before { get("/v3/repo/#{repo.id}") } before { repo.update_attribute(:private, false) } example { expect(last_response).to be_not_found } - example { expect(JSON.load(body)).to be == { + example { expect(parsed_body).to be == { "@type" => "error", "error_type" => "not_found", "error_message" => "repository not found (or insufficient access)", @@ -90,7 +91,7 @@ describe Travis::API::V3::Services::Repository::Find do before { get("/v3/repo/#{repo.id}", {}, headers) } after { repo.update_attribute(:private, false) } example { expect(last_response).to be_ok } - example { expect(JSON.load(body)).to be == { + example { expect(parsed_body).to be == { "@type" => "repository", "@href" => "/v3/repo/#{repo.id}", "id" => repo.id, @@ -136,7 +137,7 @@ describe Travis::API::V3::Services::Repository::Find do before { get("/v3/repo/#{repo.id}", {}, headers) } before { repo.update_attribute(:private, false) } example { expect(last_response).to be_not_found } - example { expect(JSON.load(body)).to be == { + example { expect(parsed_body).to be == { "@type" => "error", "error_type" => "not_found", "error_message" => "repository not found (or insufficient access)", @@ -159,7 +160,7 @@ describe Travis::API::V3::Services::Repository::Find do example { expect(last_response).to be_ok } - example { expect(JSON.load(body)).to be == { + example { expect(parsed_body).to be == { "@type" => "repository", "@href" => "/v3/repo/#{repo.id}", "id" => repo.id, @@ -211,7 +212,7 @@ describe Travis::API::V3::Services::Repository::Find do before { repo.update_attribute(:private, false) } example { expect(last_response).to be_not_found } - example { expect(JSON.load(body)).to be == { + example { expect(parsed_body).to be == { "@type" => "error", "error_type" => "not_found", "error_message" => "repository not found (or insufficient access)", @@ -234,7 +235,7 @@ describe Travis::API::V3::Services::Repository::Find do example { expect(last_response).to be_ok } - example { expect(JSON.load(body)).to be == { + example { expect(parsed_body).to be == { "@type" => "repository", "@href" => "/v3/repo/#{repo.id}", "id" => repo.id, @@ -272,4 +273,55 @@ describe Travis::API::V3::Services::Repository::Find do "finished_at" => nil}} }} end + + describe "including full owner" do + before { get("/v3/repo/#{repo.id}?include=repository.owner") } + example { expect(last_response).to be_ok } + example { expect(parsed_body['owner']).to include("github_id", "is_syncing", "synced_at", + "@type" => "user", + "id" => repo.owner_id, + "login" => "svenfuchs", + )} + end + + describe "including full owner and full last build" do + before { get("/v3/repo/#{repo.id}?include=repository.owner,repository.last_build") } + example { expect(last_response).to be_ok } + example { expect(parsed_body['last_build']['state']).to be == 'configured' } + example { expect(parsed_body['last_build']['repository']).to be == { "@href" => "/v3/repo/#{repo.id}" } } + example { expect(parsed_body['owner']).to include("github_id", "is_syncing", "synced_at")} + end + + describe "including non-existing field" do + before { get("/v3/repo/#{repo.id}?include=repository.owner,repository.last_build_number") } + example { expect(last_response.status).to be == 400 } + example { expect(parsed_body).to be == { + "@type" => "error", + "error_type" => "wrong_params", + "error_message" => "no field \"repository.last_build_number\" to include" + }} + end + + describe "wrong include format" do + before { get("/v3/repo/#{repo.id}?include=repository.last_build.branch") } + example { expect(last_response.status).to be == 400 } + example { expect(parsed_body).to be == { + "@type" => "error", + "error_type" => "wrong_params", + "error_message" => "illegal format for include parameter" + }} + end + + describe "including nested objects" do + before { get("/v3/repo/#{repo.id}?include=repository.last_build,build.branch") } + example { expect(last_response).to be_ok } + example { expect(parsed_body).to include("last_build") } + example { expect(parsed_body['last_build']).to include("branch" => { + "@type" => "branch", + "@href" => "/v3/repo/#{repo.id}/branch/master", + "name" => "master", + "repository" => { "@href" =>"/v3/repo/#{repo.id}" }, + "last_build" => { "@href" =>"/v3/build/#{repo.last_build.id}" } + }) } + end end \ No newline at end of file