Skip to content

Commit b30ba28

Browse files
adamruzickaclaude
andauthored
* Fixes #39464 - Stop serializing all hosts and template invocations in job invocation
* Fixes #39464 - Stop serializing all hosts in show Api::V2::JobInvocationsController#show endpoint serialized every host (via hosts/base) and every template invocation on each request, including the 1-second polling requests from the detail page. None of this data is used by the frontend — hosts are fetched separately by the host table, and per-host template invocations are fetched on row expand. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Refs #39464 - Clean up after removing hosts from show - Remove dead @hosts assignment from create action - Extract @pattern_template_invocations into a before_action - Add test assertion for pattern_template_invocations presence Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Refs #39464 - Inline pattern_template_invocations in show The before_action was only used by the show action, so inline the assignment rather than extracting a separate method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Refs #39464 - Add include_hosts param to avoid breaking public API Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Refs #39464 - Pass include_hosts=false from the frontend Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Refs #39464 - Improve test coverage and extract include_hosts? helper - Extract duplicated include_hosts? condition into a private method - Restore dropped 'should see only permitted hosts' authorization test - Add test for host_status=true with include_hosts=false (silent discard) - Add test for host_status=true with include_hosts=true (job_status present) - Merge redundant 'should include hosts by default' into 'should get invocation detail' Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ed77077 commit b30ba28

4 files changed

Lines changed: 81 additions & 21 deletions

File tree

app/controllers/api/v2/job_invocations_controller.rb

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,16 @@ def index
1919

2020
api :GET, '/job_invocations/:id', N_('Show job invocation')
2121
param :id, :identifier, :required => true
22-
param :host_status, :bool, required: false, desc: N_('Show Job status for the hosts')
22+
param :include_hosts, :bool, :required => false, :default_value => true, :desc => N_('Include hosts and template invocations in the response. Defaults to true for backwards compatibility. Pass false to skip serializing all hosts.')
23+
param :host_status, :bool, :required => false, :default_value => false, :desc => N_('Show job status for each host, only applicable when include_hosts is true')
2324
def show
24-
set_hosts_and_template_invocations
25+
@pattern_template_invocations = @job_invocation.pattern_template_invocations.includes(:input_values)
2526
@job_organization = Taxonomy.find_by(id: @job_invocation.task.input[:current_organization_id])
2627
@job_location = Taxonomy.find_by(id: @job_invocation.task.input[:current_location_id])
27-
if params[:host_status] == 'true'
28-
set_statuses_and_smart_proxies
28+
29+
if include_hosts?
30+
set_hosts_and_template_invocations
31+
set_statuses_and_smart_proxies if Foreman::Cast.to_bool(params[:host_status])
2932
end
3033
end
3134

@@ -84,13 +87,14 @@ def show
8487

8588
api :POST, '/job_invocations/', N_('Create a job invocation')
8689
param_group :job_invocation, :as => :create
90+
param :include_hosts, :bool, :required => false, :default_value => true, :desc => N_('Include hosts and template invocations in the response. Defaults to true for backwards compatibility. Pass false to skip serializing all hosts.')
8791
def create
8892
composer = JobInvocationComposer.from_api_params(
8993
job_invocation_params
9094
)
9195
composer.trigger!
9296
@job_invocation = composer.job_invocation
93-
@hosts = @job_invocation.targeting.hosts
97+
set_hosts_and_template_invocations if include_hosts?
9498
process_response @job_invocation
9599
rescue JobInvocationComposer::JobTemplateNotFound, JobInvocationComposer::FeatureNotFound => e
96100
not_found(error: { message: e.message })
@@ -298,7 +302,6 @@ def parent_scope
298302
end
299303

300304
def set_hosts_and_template_invocations
301-
@pattern_template_invocations = @job_invocation.pattern_template_invocations.includes(:input_values)
302305
@hosts = @job_invocation.targeting.hosts.authorized(:view_hosts, Host)
303306

304307
if params[:search].present?
@@ -310,6 +313,10 @@ def set_hosts_and_template_invocations
310313
.includes(:input_values)
311314
end
312315

316+
def include_hosts?
317+
params[:include_hosts].nil? || Foreman::Cast.to_bool(params[:include_hosts])
318+
end
319+
313320
def set_statuses_and_smart_proxies
314321
template_invocations = @template_invocations.where(host_id: @hosts.select(:id))
315322
.includes(:run_host_job_task).to_a

app/views/api/v2/job_invocations/main.json.rabl

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@ child :targeting do
1919
attributes :bookmark_id, :bookmark_name, :search_query, :targeting_type, :user_id, :status, :status_label,
2020
:randomized_ordering
2121

22-
child @hosts => :hosts do
23-
extends 'api/v2/hosts/base'
22+
if @hosts
23+
child @hosts => :hosts do
24+
extends 'api/v2/hosts/base'
2425

25-
if params[:host_status] == 'true'
26-
node :job_status do |host|
27-
@host_statuses[host.id]
26+
if @host_statuses
27+
node :job_status do |host|
28+
@host_statuses[host.id]
29+
end
2830
end
2931
end
3032
end
@@ -34,12 +36,14 @@ child :task do
3436
attributes :id, :state, :started_at
3537
end
3638

37-
child @template_invocations do
38-
attributes :template_id, :template_name, :host_id
39-
child :input_values do
40-
attributes :template_input_name, :template_input_id
41-
node :value do |iv|
42-
iv.template_input.respond_to?(:hidden_value) && iv.template_input.hidden_value? ? '*' * 5 : iv.value
39+
if @template_invocations
40+
child @template_invocations => :template_invocations do
41+
attributes :template_id, :template_name, :host_id
42+
child :input_values do
43+
attributes :template_input_name, :template_input_id
44+
node :value do |iv|
45+
iv.template_input.respond_to?(:hidden_value) && iv.template_input.hidden_value? ? '*' * 5 : iv.value
46+
end
4347
end
4448
end
4549
end

test/functional/api/v2/job_invocations_controller_test.rb

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ class JobInvocationsControllerTest < ActionController::TestCase
2727
template = ActiveSupport::JSON.decode(@response.body)
2828
assert_not_empty template
2929
assert_equal template['job_category'], @invocation.job_category
30-
assert_not_empty template['targeting']['hosts']
30+
assert_not_nil template['targeting']
31+
assert_not_empty template['pattern_template_invocations']
32+
assert_not_nil template['targeting']['hosts']
33+
assert_not_nil template['template_invocations']
3134
end
3235

3336
test 'should get invocation detail when taxonomies are set' do
@@ -36,9 +39,17 @@ class JobInvocationsControllerTest < ActionController::TestCase
3639
assert_response :success
3740
end
3841

42+
test 'should exclude hosts when include_hosts=false' do
43+
get :show, params: { :id => @invocation.id, :include_hosts => false }
44+
assert_response :success
45+
result = ActiveSupport::JSON.decode(@response.body)
46+
assert_nil result['targeting']['hosts']
47+
assert_nil result['template_invocations']
48+
end
49+
3950
test 'should see only permitted hosts' do
40-
@user = FactoryBot.create(:user, admin: false)
41-
@invocation.task.update(user: @user)
51+
@user = FactoryBot.create(:user, :admin => false)
52+
@invocation.task.update(:user => @user)
4253
setup_user('view', 'job_invocations', nil, @user)
4354
setup_user('view', 'hosts', 'name ~ nope.example.com', @user)
4455

@@ -47,6 +58,29 @@ class JobInvocationsControllerTest < ActionController::TestCase
4758
response = ActiveSupport::JSON.decode(@response.body)
4859
assert_equal response['targeting']['hosts'], []
4960
end
61+
62+
test 'should ignore host_status when include_hosts=false' do
63+
get :show, params: { :id => @invocation.id, :include_hosts => false, :host_status => true }
64+
assert_response :success
65+
result = ActiveSupport::JSON.decode(@response.body)
66+
assert_nil result['targeting']['hosts']
67+
assert_nil result['template_invocations']
68+
end
69+
70+
test 'should include job_status per host when host_status=true' do
71+
invocation = FactoryBot.create(:job_invocation, :with_template, :with_task)
72+
invocation.template_invocations << FactoryBot.create(:template_invocation, :with_task, :with_host, :job_invocation => invocation)
73+
invocation.job_category = invocation.pattern_template_invocations.first.template.job_category
74+
invocation.targeting.hosts = invocation.template_invocations.map(&:host)
75+
invocation.save!
76+
77+
get :show, params: { :id => invocation.id, :include_hosts => true, :host_status => true }
78+
assert_response :success
79+
result = ActiveSupport::JSON.decode(@response.body)
80+
hosts = result['targeting']['hosts']
81+
assert_not_empty hosts
82+
assert hosts.all? { |h| h.key?('job_status') }, 'Expected job_status to be present on each host'
83+
end
5084
end
5185

5286
context 'creation' do
@@ -66,6 +100,20 @@ class JobInvocationsControllerTest < ActionController::TestCase
66100
assert_response :success
67101
end
68102

103+
test 'should include hosts in create response by default' do
104+
post :create, params: { job_invocation: @attrs }
105+
invocation = ActiveSupport::JSON.decode(@response.body)
106+
assert_not_nil invocation['targeting']['hosts']
107+
assert_response :success
108+
end
109+
110+
test 'should exclude hosts from create response when include_hosts=false' do
111+
post :create, params: { job_invocation: @attrs, include_hosts: false }
112+
invocation = ActiveSupport::JSON.decode(@response.body)
113+
assert_nil invocation['targeting']['hosts']
114+
assert_response :success
115+
end
116+
69117
test 'should create with description format overridden' do
70118
@attrs[:description_format] = 'format'
71119
post :create, params: { job_invocation: @attrs }

webpack/JobInvocationDetail/JobInvocationActions.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export const getJobInvocation = url => dispatch => {
1919
const fetchData = withInterval(
2020
get({
2121
key: JOB_INVOCATION_KEY,
22-
params: { include_permissions: true },
22+
params: { include_permissions: true, include_hosts: false },
2323
url,
2424
handleError: () => {
2525
dispatch(stopInterval(JOB_INVOCATION_KEY));
@@ -44,6 +44,7 @@ export const updateJob = jobId => dispatch => {
4444
APIActions.get({
4545
url,
4646
key: UPDATE_JOB,
47+
params: { include_hosts: false },
4748
})
4849
);
4950
};

0 commit comments

Comments
 (0)