From 525fd78cd711f4cadf8c50296bb35cc2f57c9c27 Mon Sep 17 00:00:00 2001 From: Jeremy Lenz Date: Fri, 15 Sep 2023 12:57:02 -0400 Subject: [PATCH] Refs #36715 - don't add loader; switch to partha approach --- app/models/host/base.rb | 37 +++---------------- app/models/host/managed.rb | 2 +- .../foreman/renderer/scope/macros/loaders.rb | 7 ---- test/models/host_jail_test.rb | 2 +- 4 files changed, 8 insertions(+), 40 deletions(-) diff --git a/app/models/host/base.rb b/app/models/host/base.rb index bdcd64d5c04c..2757b5d7184f 100644 --- a/app/models/host/base.rb +++ b/app/models/host/base.rb @@ -47,10 +47,6 @@ class Base < ApplicationRecord default_scope -> { where(taxonomy_conditions) } - def self.with_facts - includes(:fact_names, :fact_values) - end - def self.taxonomy_conditions conditions = {} if Organization.current.nil? && User.current.present? && !User.current.admin? @@ -218,45 +214,24 @@ def set_comment(parser) desc 'Note that available facts depend on what facts have been uploaded to Foreman, typical sources are Puppet facter, subscription manager etc. The facts can be out of date, this macro only provides access to the value stored in the database.' + optional :fact_names, Array, desc: 'A list of fact names to return. If empty all facts are returned' returns Hash, desc: 'A hash of facts, keys are fact names, values are fact values' example '@host.facts # => { "hardwareisa"=>"x86_64", "kernel"=>"Linux", "virtual"=>"physical", ... }', desc: 'Getting all host facts' example '@host.facts["uptime"] # => "30 days"', desc: 'Getting specific fact value, +uptime+ in this case' aliases :facts end - def facts_hash + def facts_hash(fact_names = [], *args) + filtered_names = [fact_names, *args].flatten.compact hash = {} - fact_values.includes(:fact_name).collect do |fact| + query = fact_values.includes(:fact_name) + query = query.where(fact_names: {name: filtered_names}) if filtered_names.present? + query.each do |fact| hash[fact.fact_name.name] = fact.value end hash end alias_method :facts, :facts_hash - apipie :method, 'List only specific facts about the host.' do - desc 'Same as +facts+ macro, but only returns facts with given names. - If fact_names and fact_values are already loaded via :includes, this macro will not perform any additional queries.' - returns Hash, desc: 'A hash of facts, keys are fact names, values are fact values' - example '@host.facts_with_names([\'dmi::memory::size\', \'net::interface::eth0\']) # => { "dmi::memory::size"=>"16 GB", "net::interface::eth0"=>"00:50:56:8a:5c:3c" }', desc: 'Getting specific facts' - end - def facts_with_names(names_query = [], *args) - filtered_names = Set.new([names_query, *args].flatten.compact) - result = {} - names = {} # { fact_name_id => fact_name_name } - fact_names.each do |fact_name| - next unless filtered_names.empty? || filtered_names.include?(fact_name.name) - names[fact_name.id] = fact_name.name - end - f_values = {} # { fact_name_id => fact_value_value } - fact_values.each do |fact_value| - next unless names[fact_value.fact_name_id] - f_values[fact_value.fact_name_id] = fact_value.value - end - names.each do |id, name| - result[name] = f_values[id] - end - result - end - def ==(comparison_object) super || comparison_object.is_a?(Host::Base) && diff --git a/app/models/host/managed.rb b/app/models/host/managed.rb index a42e0cb7eeb8..369d2df0bfa9 100644 --- a/app/models/host/managed.rb +++ b/app/models/host/managed.rb @@ -169,7 +169,7 @@ class Jail < ::Safemode::Jail :model, :certname, :capabilities, :provider, :subnet, :subnet6, :token, :location, :organization, :provision_method, :image_build?, :pxe_build?, :otp, :realm, :nil?, :indent, :primary_interface, :provision_interface, :interfaces, :bond_interfaces, :bridge_interfaces, :interfaces_with_identifier, - :managed_interfaces, :facts, :facts_hash, :facts_with_names, :root_pass, :sp_name, :sp_ip, :sp_mac, :sp_subnet, :use_image, + :managed_interfaces, :facts, :facts_hash, :root_pass, :sp_name, :sp_ip, :sp_mac, :sp_subnet, :use_image, :multiboot, :jumpstart_path, :install_path, :miniroot, :medium, :bmc_nic, :templates_used, :owner, :owner_type, :ssh_authorized_keys, :pxe_loader, :global_status, :global_status_label, :get_status, :puppetca_token, :last_report, :build?, :smart_proxies, :host_param, :virtual, :ram, :sockets, :cores, :params, :pxe_loader_efi?, :comment diff --git a/app/services/foreman/renderer/scope/macros/loaders.rb b/app/services/foreman/renderer/scope/macros/loaders.rb index db7b12c7617d..ba930efb3a26 100644 --- a/app/services/foreman/renderer/scope/macros/loaders.rb +++ b/app/services/foreman/renderer/scope/macros/loaders.rb @@ -70,13 +70,6 @@ module Loaders end end - def load_hosts_with_facts(search:, batch: 1_000, includes: nil, limit: nil, select: nil, joins: nil, where: nil, preload: nil) - incl = [:fact_values, :fact_names] + (includes || []) - load_resource(klass: Host, permission: :view_hosts, search: search, batch: batch, includes: incl, limit: limit, select: select, joins: joins, where: where, preload: preload) - end - - LOADERS << [:load_hosts_with_facts] - private # returns a batched relation, use either diff --git a/test/models/host_jail_test.rb b/test/models/host_jail_test.rb index 2c517db5482c..c2f8899ce30e 100644 --- a/test/models/host_jail_test.rb +++ b/test/models/host_jail_test.rb @@ -6,7 +6,7 @@ def test_jail_should_include_these_methods :organization, :url_for_boot, :hostgroup, :compute_resource, :domain, :ip, :mac, :shortname, :architecture, :model, :certname, :capabilities, :provider, :subnet, :token, :location, :organization, :provision_method, :image_build?, :pxe_build?, :otp, :realm, :nil?, :indent, :sp_name, :sp_ip, :sp_mac, :sp_subnet, :facts, - :facts_with_names, :facts_hash, :bmc_nic, :templates_used, :owner, :owner_type, :ssh_authorized_keys, :last_report] + :facts_hash, :bmc_nic, :templates_used, :owner, :owner_type, :ssh_authorized_keys, :last_report] allowed.each do |m| assert Host::Managed::Jail.allowed?(m), "Method #{m} is not available in Host::Managed::Jail while should be allowed."