Skip to content

Commit 886dd97

Browse files
committed
Add include_for_find support via 'expand'
Allows for opt in of `:include_for_find` option for `Rbac::Filterer` via the `expand` request parameter. Include for find will effectively add a `.include` to the base query done in `collection_search`, which in cases like this: /api/vms?expand=resources,hardware&attributes=num_cpu,name Can avoid an N+1 on the associated resource.
1 parent 0293d94 commit 886dd97

File tree

5 files changed

+49
-0
lines changed

5 files changed

+49
-0
lines changed

app/controllers/api/base_controller/parameters.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,16 @@ def sort_directive(klass, attr, order, options)
100100
end
101101
arel
102102
end
103+
104+
def determine_include_for_find(klass)
105+
return nil unless klass.respond_to?(:reflect_on_association)
106+
107+
relations = @req.derived_include_for_find.select do |relation|
108+
klass.reflect_on_association(relation)
109+
end
110+
111+
relations.empty? ? nil : relations
112+
end
103113
end
104114
end
105115
end

app/controllers/api/base_controller/renderer.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ def collection_filterer(res, type, klass, is_subcollection = false)
185185
options[:filter] = miq_expression if miq_expression
186186
options[:offset] = params['offset'] if params['offset']
187187
options[:limit] = params['limit'] if params['limit']
188+
options[:include_for_find] = determine_include_for_find(klass)
188189

189190
filter_results(miq_expression, res, options)
190191
end

lib/api/request_adapter.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ def attributes
3737
@attributes ||= @params['attributes'].to_s.split(',')
3838
end
3939

40+
def derived_include_for_find
41+
@derived_include_for_find ||= expand_requested.reject { |item| item == "resource" }
42+
end
43+
4044
def base
4145
url.partition(fullpath)[0] # http://target
4246
end

spec/requests/vms_spec.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ def update_raw_power_state(state, *vms)
8282
[{"vendor" => "openstack"},
8383
{"vendor" => "openstack"}])
8484
end
85+
86+
include_examples "for 'expand' query optimizations" do
87+
let!(:includes) { "hardware" }
88+
let!(:attributes) { "num_cpu,name" }
89+
end
8590
end
8691

8792
context 'Vm edit' do
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
RSpec.shared_context "for 'expand' query optimizations" do
2+
context "expand attribute query optimizations" do
3+
let(:resource) { :vms }
4+
let(:index_url) { api_vms_url }
5+
6+
it "removes N+1's from the index query for subcollections/virtual_attributes" do
7+
api_basic_authorize action_identifier(resource, :read, :resource_actions, :get)
8+
9+
expands = ["resources"]
10+
from_match = [resource]
11+
12+
if defined?(includes)
13+
expands << includes
14+
from_match << includes
15+
end
16+
17+
attrs = defined?(attributes) ? attributes : "resources"
18+
query_match = /SELECT.*FROM\s"(?:#{from_match.join("|")})"/m
19+
expected_query_count = defined?(query_count) ? query_count : 10
20+
21+
expect {
22+
get index_url, :params => {
23+
:expand => expands.join(','),
24+
:attributes => attrs
25+
}
26+
}.to make_database_queries(:count => expected_query_count, :matching => query_match)
27+
end
28+
end
29+
end

0 commit comments

Comments
 (0)