Skip to content

Commit

Permalink
Fixes #25188 - Remove unbound facts from /hosts/x/facts
Browse files Browse the repository at this point in the history
Revert "Fixes #20891 - Remove hostname from /hosts/x/facts results"
This reverts commit 8ebbbec.

The previous fix made the API results backward incompatible
and difficult to parse and had to be reverted.
  • Loading branch information
mbacovsky authored and tbrisker committed Oct 16, 2018
1 parent ea48efb commit f5a53ca
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 13 deletions.
3 changes: 1 addition & 2 deletions app/controllers/api/v2/fact_values_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ def index
no_timestamp_facts.
search_for(*search_options).paginate(paginate_options).
preload(:fact_name, :host)
host_id = params[:host_id]
@fact_values = FactValue.build_facts_hash(values, host_id)
@fact_values = FactValue.build_facts_hash(values.all)
end

def setup_search_options
Expand Down
11 changes: 7 additions & 4 deletions app/models/fact_value.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,13 @@ def self.count_each(fact, options = {})
output
end

def self.build_facts_hash(values, host_id = nil)
hash = values.group_by(&:host_name).transform_values! {|val| val.map {|v| [v.fact_name_name, v.value]}.to_h}
if host_id && hash.any?
hash.merge! hash.values[0]
def self.build_facts_hash(facts)
hosts = Host.where(:id => facts.group_by(&:host_id).keys).all

hash = {}
facts.each do |fact|
hash[hosts.detect {|h| h.id == fact.host_id}.to_s] ||= {}
hash[hosts.detect {|h| h.id == fact.host_id}.to_s].update({fact.name.to_s => fact.value})
end
hash
end
Expand Down
14 changes: 7 additions & 7 deletions test/controllers/api/v2/fact_values_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,26 @@ def setup
end

test "should get facts for given host only" do
get :index, params: {:host_id => @host.name}
get :index, params: { :host_id => @host.name }
assert_response :success
fact_values = ActiveSupport::JSON.decode(@response.body)['results']
expected_hash = {@host.name => {"kernelversion" => "2.6.9"}, "kernelversion" => "2.6.9"}
expected_hash = FactValue.build_facts_hash(FactValue.where(:host_id => @host.id))
assert_equal expected_hash, fact_values
end

test "should get facts for given host id" do
get :index, params: {:host_id => @host.id}
get :index, params: { :host_id => @host.id }
assert_response :success
fact_values = ActiveSupport::JSON.decode(@response.body)['results']
expected_hash = {@host.name => {"kernelversion" => "2.6.9"}, "kernelversion" => "2.6.9"}
fact_values = ActiveSupport::JSON.decode(@response.body)['results']
expected_hash = FactValue.build_facts_hash(FactValue.where(:host_id => @host.id))
assert_equal expected_hash, fact_values
end

test "should get facts as non-admin user with joined search" do
setup_user
@host.update_attribute(:hostgroup, FactoryBot.create(:hostgroup))
as_user(users(:one)) do
get :index, params: {:search => "host.hostgroup = #{@host.hostgroup.name}"}
get :index, params: { :search => "host.hostgroup = #{@host.hostgroup.name}" }
end
assert_response :success
fact_values = ActiveSupport::JSON.decode(@response.body)['results']
Expand Down Expand Up @@ -79,6 +79,6 @@ def setup

def setup_user
@request.session[:user] = users(:one).id
users(:one).roles = [Role.default, Role.find_by_name('Viewer')]
users(:one).roles = [Role.default, Role.find_by_name('Viewer')]
end
end

0 comments on commit f5a53ca

Please sign in to comment.