Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add paging links to the API #15148

Merged
merged 9 commits into from
Jul 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions app/controllers/api/base_controller/generic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,14 @@ module Generic

def index
klass = collection_class(@req.subject)
res = collection_search(@req.subcollection?, @req.subject, klass)
res, subquery_count = collection_search(@req.subcollection?, @req.subject, klass)
opts = {
:name => @req.subject,
:is_subcollection => @req.subcollection?,
:expand_actions => true,
:count => klass.count,
:expand_resources => @req.expand?(:resources),
:subcount => res.length
:counts => Api::QueryCounts.new(klass.count, res.count, subquery_count)
}

render_collection(@req.subject, res, opts)
end

Expand Down
10 changes: 0 additions & 10 deletions app/controllers/api/base_controller/parameters.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,6 @@
module Api
class BaseController
module Parameters
def paginate_params?
params['offset'] || params['limit']
end

def expand_paginate_params
offset = params['offset'] # 0 based
limit = params['limit'] # i.e. page size
[offset, limit]
end

def hash_fetch(hash, element, default = {})
hash[element] || default
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ def version
end
end

def url
@request.original_url # http://target/api/...
end

private

def expand_requested
Expand All @@ -132,10 +136,6 @@ def prefix
prefix = "/#{path.split('/')[1]}" # /api
version_override? ? "#{prefix}/#{@params[:version]}" : prefix
end

def url
@request.original_url # http://target/api/...
end
end
end
end
Expand Down
38 changes: 33 additions & 5 deletions app/controllers/api/base_controller/renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,19 @@ def gen_reftype(type, opts)
# Given a resource, return its serialized flavor using Jbuilder
#
def collection_to_jbuilder(type, reftype, resources, opts = {})
link_builder = Api::LinksBuilder.new(params, @req.url, opts[:counts])
Jbuilder.new do |json|
json.ignore_nil!
[:name, :count, :subcount].each do |opt_name|
json.set! opt_name.to_s, opts[opt_name] if opts[opt_name]
json.set! 'name', opts[:name] if opts[:name]

if opts[:counts]
opts[:counts].counts.each do |count, value|
json.set! count, value
end
end

json.set! 'pages', link_builder.pages if link_builder.links?

unless @req.hide?("resources")
json.resources resources.collect do |resource|
if opts[:expand_resources]
Expand All @@ -45,6 +53,14 @@ def collection_to_jbuilder(type, reftype, resources, opts = {})
cspec = collection_config[type]
aspecs = gen_action_spec_for_collections(type, cspec, opts[:is_subcollection], reftype) if cspec
add_actions(json, aspecs, reftype)

if link_builder.links?
json.links do
link_builder.links.each do |link_name, link_href|
json.set! link_name, link_href
end
end
end
end
end

Expand Down Expand Up @@ -156,10 +172,22 @@ def collection_search(is_subcollection, type, klass)

options = {:user => User.current_user}
options[:order] = sort_options if sort_options.present?
options[:offset], options[:limit] = expand_paginate_params if paginate_params?
options[:filter] = miq_expression if miq_expression
options[:offset] = params['offset'] if params['offset']
options[:limit] = params['limit'] if params['limit']

if miq_expression.present? && options.key?(:limit) && options.key?(:offset)
filter_results_for_paging(res, options)
else
[Rbac.filtered(res, options)]
end
end

Rbac.filtered(res, options)
# If a filter is present, first get the count for the filtered result,
# Then apply Rbac filter for paging
def filter_results_for_paging(res, options)
subquery_res = Rbac.filtered(res, options.except(:offset, :limit))
[Rbac.filtered(res, options), subquery_res.count]
end

def virtual_attribute_search(resource, attribute)
Expand Down Expand Up @@ -359,7 +387,7 @@ def validate_attr_selection(resource)
def expand_subcollection(json, sc, sctype, subresources)
if collection_config.show_as_collection?(sc)
copts = {
:count => subresources.length,
:counts => Api::QueryCounts.new(subresources.length),
:is_subcollection => true,
:expand_resources => @req.expand?(sc)
}
Expand Down
66 changes: 66 additions & 0 deletions lib/api/links_builder.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
module Api
class LinksBuilder
def initialize(params, href, counts)
@offset = params["offset"].to_i if params["offset"]
@limit = params["limit"].to_i if params["limit"]
@href = href
@counts = counts if counts
end

def links
{
:self => self_href,
:next => next_href,
:previous => previous_href,
:first => first_href,
:last => last_href
}.compact
end

def pages
(paging_count / limit.to_f).ceil
end

def links?
offset && limit
end

private

attr_reader :offset, :limit, :href, :counts

def self_href
@self_href ||= format_href(offset)
end

def format_href(new_offset)
href.sub("offset=#{offset}", "offset=#{new_offset}")
end

def paging_count
@paging_count ||= counts.subquery_count || counts.count
end

def next_href
next_offset = offset + limit
return if next_offset >= paging_count
format_href(next_offset)
end

def previous_href
return if offset.zero?
prev_offset = offset - limit
return first_href if prev_offset < 0
format_href(prev_offset)
end

def first_href
format_href(0)
end

def last_href
last_offset = paging_count - (paging_count % limit)
format_href(last_offset)
end
end
end
19 changes: 19 additions & 0 deletions lib/api/query_counts.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module Api
class QueryCounts
attr_reader :count, :subcount, :subquery_count

def initialize(count, subcount = nil, subquery_count = nil)
@count = count
@subcount = subcount
@subquery_count = subquery_count
end

def counts
{
:count => count,
:subcount => subcount,
:subquery_count => subquery_count
}.compact
end
end
end
92 changes: 92 additions & 0 deletions spec/lib/api/links_builder_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
RSpec.describe Api::LinksBuilder do
describe "#links" do
it "returns self, first, next, and last links when it is the first page" do
offsets = { "offset" => 0, "limit" => 2 }
counts = Api::QueryCounts.new(7, 2)
link_builder = Api::LinksBuilder.new(offsets, create_href(offsets), counts)
links = link_builder.links
expect(links.keys).to match_array([:self, :first, :next, :last])
expect(links[:self]).to eq(create_href(offsets))
expect(links[:next]).to eq(create_href("offset" => 2, "limit" => 2))
expect(links[:last]).to eq(create_href("offset" => 6, "limit" => 2))
end

it "returns all of the links if it is a middle page" do
offsets = { "offset" => 2, "limit" => 2 }
counts = Api::QueryCounts.new(7, 2)
link_builder = Api::LinksBuilder.new(offsets, create_href(offsets), counts)
links = link_builder.links

expect(links.keys).to match_array([:self, :next, :previous, :first, :last])
expect(links[:self]).to eq(create_href(offsets))
expect(links[:next]).to eq(create_href("offset" => 4, "limit" => 2))
expect(links[:previous]).to eq(create_href("offset" => 0, "limit" => 2))
expect(links[:first]).to eq(create_href("offset" => 0, "limit" => 2))
expect(links[:last]).to eq(create_href("offset" => 6, "limit" => 2))
end

it "returns self, previous, first, last if it is the last page" do
offsets = { "offset" => 3, "limit" => 2 }
counts = Api::QueryCounts.new(7, 2, 3)
link_builder = Api::LinksBuilder.new(offsets, create_href(offsets), counts)
links = link_builder.links
expect(links.keys).to eq([:self, :previous, :first, :last])
expect(links[:self]).to eq(create_href(offsets))
expect(links[:previous]).to eq(create_href("offset" => 1, "limit" => 2))
expect(links[:first]).to eq(create_href("offset" => 0, "limit" => 2))
expect(links[:last]).to eq(create_href("offset" => 2, "limit" => 2))
end

it "always returns self, first, and last" do
offsets = { "offset" => 0, "limit" => 3 }
counts = Api::QueryCounts.new(7, 3, 3)
link_builder = Api::LinksBuilder.new(offsets, create_href(offsets), counts)
links = link_builder.links

expect(links.keys).to match_array([:self, :first, :last])
expect(links[:self]).to eq(create_href(offsets))
end

it "previous link is equal to first link if previous offset would be 0" do
offsets = { "offset" => 10, "limit" => 12 }
counts = Api::QueryCounts.new(22, 22, 22)
link_builder = Api::LinksBuilder.new(offsets, create_href(offsets), counts)
links = link_builder.links

expect(links[:previous]).to eq(links[:first])
end
end

describe "#pages" do
let(:offsets) { { "offset" => 0, "limit" => 2 } }

it "returns correct page count when last page count is less than the limit" do
counts = Api::QueryCounts.new(7, 2)
link_builder = Api::LinksBuilder.new(offsets, create_href(offsets), counts)
expect(link_builder.pages).to eq(4)
end

it "returns correct page count when there is only one page" do
counts = Api::QueryCounts.new(7, 2, 2)
link_builder = Api::LinksBuilder.new(offsets, create_href(offsets), counts)
expect(link_builder.pages).to eq(1)
end

it "returns correct page count when there are no subquery results" do
counts = Api::QueryCounts.new(7, 2, 0)
link_builder = Api::LinksBuilder.new(offsets, create_href(offsets), counts)
expect(link_builder.pages).to eq(0)
end

it "returns the correct page count when last page count is equal to the limit" do
offsets = { "offset" => 0, "limit" => 3 }
counts = Api::QueryCounts.new(6, 3)
link_builder = Api::LinksBuilder.new(offsets, create_href(offsets), counts)
expect(link_builder.pages).to eq(2)
end
end

def create_href(offsets)
"/api/vms?filter[]=name='aa'&offset=#{offsets["offset"]}&limit=#{offsets["limit"]}"
end
end
Loading