Fix host creation resource origin

* Add new resource origin ComputeAttributesOrigin (available during host
  creation)
* In ResourceOrigins, iterate hosts by name instead of id (new host has
  no id)
* Add resource quota exception classes
* Add host managed extensions test
* Add host managed factory extensions
* Fix resource to string and tests
* Fix deepCopy null error
* Fix autoload filepaths

app/helpers/foreman_resource_quota/resource_quota_helper.rb

@@ -2,40 +2,118 @@
 
 module ForemanResourceQuota
   module ResourceQuotaHelper
+    include ResourceOrigin
+    FACTOR_B_TO_KB = 1024
     FACTOR_B_TO_MB = 1024 * 1024
     FACTOR_B_TO_GB = 1024 * FACTOR_B_TO_MB
 
-    def natural_resource_name_by_type(type)
-      type_names = { cpu_cores: 'CPU cores', memory_mb: 'Memory (MB)', disk_gb: 'Disk space (GB)' }
-      type_names[type]
+    def natural_resource_name_by_type(resource_type)
+      type_names = { cpu_cores: 'CPU cores', memory_mb: 'Memory', disk_gb: 'Disk space' }
+      key = resource_type.is_a?(String) ? resource_type.to_sym : resource_type
+      raise "No natural name for unknown resource type '#{resource_type}'" unless type_names.key?(key)
+      type_names[key]
     end
 
-    def build_missing_resources_per_host_list(hosts, quota_utilization)
-      # missing_res_per_host := { <host_id>: [<list of to be determined resources>] }
-      # for example: { 1: [ :disk_gb ], 2: [ :cpu_cores, :disk_gb ] }
-      return {} if hosts.empty? || quota_utilization.empty?
+    def units_by_type(resource_type)
+      type_units = {
+        cpu_cores: [
+          { symbol: 'cores', factor: 1 },
+        ],
+        memory_mb: [
+          { symbol: 'MB', factor: 1 },
+          { symbol: 'GB', factor: FACTOR_B_TO_KB },
+          { symbol: 'TB', factor: FACTOR_B_TO_MB },
+        ],
+        disk_gb: [
+          { symbol: 'GB', factor: 1 },
+          { symbol: 'TB', factor: FACTOR_B_TO_KB },
+          { symbol: 'PB', factor: FACTOR_B_TO_MB },
+        ],
+      }
+      key = resource_type.is_a?(String) ? resource_type.to_sym : resource_type
+      raise "No units for unknown resource type '#{resource_type}'" unless type_units.key?(key)
+      type_units[key]
+    end
 
-      missing_res_per_host = hosts.map(&:id).index_with { [] }
-      missing_res_per_host.each_key do |host_id|
-        quota_utilization.each do |resource|
-          missing_res_per_host[host_id] << resource unless resource.nil?
-        end
+    def find_largest_unit(resource_value, units)
+      units.reverse_each do |unit|
+        return unit.values_at(:symbol, :factor) if resource_value >= unit[:factor]
       end
-      missing_res_per_host
+      units[0].values_at(:symbol, :factor)
+    end
+
+    def resource_value_to_string(resource_value, resource_type)
+      (symbol, factor) = find_largest_unit(resource_value, units_by_type(resource_type))
+      unit_applied_value = (resource_value / factor).round(1)
+      format_text = if (unit_applied_value % 1).zero?
+                      '%.0f %s'
+                    else
+                      '%.1f %s'
+                    end
+      format(format_text, unit_applied_value, symbol)
     end
 
     def utilization_from_resource_origins(resources, hosts, use_compute_resource: true, use_vm_attributes: true,
-                                          use_facts: true)
-      utilization = resources.each.with_object({}) { |key, hash| hash[key] = 0 }
-      missing_res_per_host = build_missing_resources_per_host_list(hosts, resources)
+                                          use_compute_attributes: true, use_facts: true)
+      utilization_sum = resources.each.with_object({}) { |key, hash| hash[key] = 0 }
+      missing_hosts_resources = create_missing_hosts_resources_hash(hosts, resources)
+      hosts_hash = hosts.index_by(&:name)
 
       if use_compute_resource
-        ResourceOrigin::ComputeResourceOrigin.new.collect_resources!(utilization, missing_res_per_host)
+        ResourceOrigin::ComputeResourceOrigin.new.collect_resources!(
+          utilization_sum,
+          missing_hosts_resources,
+          hosts_hash
+        )
+      end
+      if use_vm_attributes
+        ResourceOrigin::VMAttributesOrigin.new.collect_resources!(
+          utilization_sum,
+          missing_hosts_resources,
+          hosts_hash
+        )
+      end
+      if use_compute_attributes
+        ResourceOrigin::ComputeAttributesOrigin.new.collect_resources!(
+          utilization_sum,
+          missing_hosts_resources,
+          hosts_hash
+        )
+      end
+      if use_facts
+        ResourceOrigin::FactsOrigin.new.collect_resources!(
+          utilization_sum,
+          missing_hosts_resources,
+          hosts_hash
+        )
       end
-      ResourceOrigin::VMAttributesOrigin.new.collect_resources!(utilization, missing_res_per_host) if use_vm_attributes
-      ResourceOrigin::FactsOrigin.new.collect_resources!(utilization, missing_res_per_host) if use_facts
 
-      [utilization, missing_res_per_host]
+      [utilization_sum, missing_hosts_resources]
+    end
+
+    private
+
+    # Create a hash that maps resources to host names.
+    # Parameters:
+    #   - hosts: An array of host objects.
+    #   - resources: List of resources to be determined per host.
+    # Returns:
+    #   - missing_hosts_resources := { <host name>: [<list of to be determined resources>] }
+    #     for example:
+    #     {
+    #       "host_a": {
+    #         [ :cpu_cores, :disk_gb ]
+    #       },
+    #       "host_b": {
+    #         [ :cpu_cores, :disk_gb ]
+    #       },
+    def create_missing_hosts_resources_hash(hosts, resources)
+      return {} if hosts.empty? || resources.empty?
+
+      resources_to_determine = resources.compact
+      return {} if resources_to_determine.empty?
+
+      hosts.map(&:name).index_with { resources_to_determine.clone }
     end
   end
 end

app/lib/foreman_resource_quota/exceptions.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module ForemanResourceQuota
+  module Exceptions
+    class HostResourceQuotaEmptyException < Foreman::Exception
+    end
+
+    class ResourceLimitException < Foreman::Exception
+    end
+
+    class HostResourcesException < Foreman::Exception
+    end
+
+    class ResourceQuotaUtilizationException < Foreman::Exception
+    end
+  end
+end

app/models/concerns/foreman_resource_quota/host_managed_extensions.rb

@@ -3,7 +3,8 @@
 module ForemanResourceQuota
   module HostManagedExtensions
     extend ActiveSupport::Concern
-    include ResourceQuotaHelper
+    include ForemanResourceQuota::ResourceQuotaHelper
+    include ForemanResourceQuota::Exceptions
 
     included do
       validate :check_resource_quota_capacity
@@ -12,50 +13,80 @@ module HostManagedExtensions
       scoped_search relation: :resource_quota, on: :name, complete_value: true, rename: :resource_quota
     end
 
-    # rubocop: disable Metrics/AbcSize
     def check_resource_quota_capacity
-      return errors.empty? if early_return?
+      return true if early_return?
 
-      resource_quota.determine_utilization([self])
-      unless resource_quota.missing_hosts.empty?
-        errors.add(:resource_quota,
-          N_(format('An error occured reading host resources. Check the foreman error log.')))
-        return false
-      end
-
-      verify_resource_quota_limits(resource_quota.utilization)
-      errors.empty?
+      quota_utilization = determine_quota_utilization
+      host_resources = determine_host_resources
+      verify_resource_quota_limits(quota_utilization, host_resources)
+      true
+    rescue Foreman::Exception => e
+      Rails.logger.error(N_(format('An error occured while checking the resource quota capacity: %s', e)))
+      errors.add(:resource_quota, e.bare_message)
+      false
+    rescue StandardError => e
+      Rails.logger.error(N_(format('An unknown error occured while checking the resource quota capacity: %s', e)))
+      errors.add(:base, e.message)
+      false
     end
-    # rubocop: enable Metrics/AbcSize
 
     private
 
-    def verify_resource_quota_limits(quota_utilization)
-      quota_utilization.each do |type, utilization|
-        next if utilization.nil?
-        max_quota = resource_quota[type]
-        next if utilization <= max_quota
-        errors.add(:resource_quota, N_(format("Host resources exceed quota '%s' for %s: %s > %s",
-          resource_quota_name,
-          natural_resource_name_by_type(type),
-          utilization,
-          max_quota)))
-        break
+    def verify_resource_quota_limits(quota_utilization, host_resources)
+      quota_utilization.each do |resource_type, resource_utilization|
+        next if resource_utilization.nil?
+
+        max_quota = resource_quota[resource_type]
+        all_hosts_utilization = resource_utilization + host_resources[resource_type.to_sym]
+        next if all_hosts_utilization <= max_quota
+
+        raise ResourceLimitException, formulate_limit_error(resource_utilization,
+          all_hosts_utilization, max_quota, resource_type)
+      end
+    end
+
+    def determine_host_resources
+      (host_resources, missing_hosts) = utilization_from_resource_origins(resource_quota.active_resources, [self])
+      unless missing_hosts.empty?
+        raise HostResourcesException,
+          "Cannot determine host resources for #{missing_hosts}"
       end
+      host_resources
+    end
+
+    def determine_quota_utilization
+      resource_quota.determine_utilization
+      missing_hosts = resource_quota.missing_hosts
+      unless missing_hosts.empty?
+        raise UtilizationException,
+          "Resource Quota '#{resource_quota.name}' cannot determine resources for #{missing_hosts.size} hosts."
+      end
+      resource_quota.utilization
+    end
+
+    def formulate_limit_error(resource_utilization, all_hosts_utilization, max_quota, resource_type)
+      error_text = if resource_utilization < max_quota
+                     "Host exceeds %s limit of '%s'-quota by %s (max. %s)"
+                   else
+                     "%s limit of '%s'-quota exceeded by %s (max. %s)"
+                   end
+      N_(format(error_text, natural_resource_name_by_type(resource_type),
+        resource_quota.name,
+        resource_value_to_string(all_hosts_utilization - max_quota, resource_type),
+        resource_value_to_string(max_quota, resource_type)))
     end
 
     def early_return?
       if resource_quota.nil?
         return true if quota_assigment_optional?
-        errors.add(:resource_quota, 'must be given.')
-        return true
+        raise HostResourceQuotaEmptyException, 'must be given.'
       end
       return true if Setting[:resource_quota_global_no_action] # quota is assigned, but not supposed to be checked
       false
     end
 
     def quota_assigment_optional?
-      owner.resource_quota_is_optional || owner.admin || Setting[:resource_quota_global_optional_user_assignment]
+      owner.resource_quota_is_optional || Setting[:resource_quota_optional_assignment]
     end
   end
 end

app/models/foreman_resource_quota/resource_quota.rb

@@ -38,7 +38,7 @@ def number_of_usergroups
       usergroups.size
     end
 
-    def determine_utilization(additional_hosts: [])
+    def determine_utilization(additional_hosts = [])
       quota_hosts = (hosts | (additional_hosts))
       self.utilization, self.missing_hosts = call_utilization_helper(quota_hosts)
 
@@ -52,13 +52,6 @@ def to_label
       name
     end
 
-    private
-
-    # Wrap into a function for better testing
-    def call_utilization_helper(quota_hosts)
-      utilization_from_resource_origins(active_resources, quota_hosts)
-    end
-
     def active_resources
       resources = []
       %i[cpu_cores memory_mb disk_gb].each do |res|
@@ -67,11 +60,18 @@ def active_resources
       resources
     end
 
+    private
+
+    # Wrap into a function for better testing
+    def call_utilization_helper(quota_hosts)
+      utilization_from_resource_origins(active_resources, quota_hosts)
+    end
+
     def print_warning(missing_hosts, hosts)
-      warn_text = "Could not determines resources for #{utilization_hash.missing_hosts.size} hosts:"
+      warn_text = "Could not determines resources for #{missing_hosts.size} hosts:"
       missing_hosts.each do |host_id, missing_resources|
         missing_host = hosts.find { |obj| obj.id == host_id }
-        warn_text << "  '#{missing_host.name}': '#{missing_resources}'\n"
+        warn_text << "  '#{missing_host.name}': '#{missing_resources}'\n" unless missing_host.nil?
       end
       Rails.logger.warn warn_text
     end

app/services/foreman_resource_quota/resource_origin.rb

@@ -9,22 +9,23 @@ class ResourceOrigin
         disk_gb: :extract_disk_gb,
       }.freeze
 
-      def collect_resources!(resources_sum, missing_res_per_host)
-        return if missing_res_per_host.empty?
+      def collect_resources!(resources_sum, missing_hosts_resources, host_objects)
+        return if missing_hosts_resources.empty?
 
-        relevant_hosts = Host::Managed.where(id: missing_res_per_host.keys).includes(host_eager_name)
-        relevant_hosts = relevant_hosts.compact
+        relevant_hosts = load_hosts_eagerly(missing_hosts_resources, host_objects, host_attribute_eager_name)
         host_values = collect_attribute_from_hosts(relevant_hosts, host_attribute_name)
-        host_values.each do |host_id, attribute_content|
-          missing_res_per_host[host_id].reverse_each do |resource_name|
-            process_resource!(resources_sum, missing_res_per_host, resource_name, host_id,
-              attribute_content)
+        host_values.each do |host_name, attribute_content|
+          missing_hosts_resources[host_name].reverse_each do |resource_name|
+            resource_value = process_resource(resource_name, attribute_content)
+            next unless resource_value
+            resources_sum[resource_name] += resource_value
+            missing_hosts_resources[host_name].delete(resource_name)
           end
-          missing_res_per_host.delete(host_id) if missing_res_per_host[host_id].empty?
+          missing_hosts_resources.delete(host_name) if missing_hosts_resources[host_name].empty?
         end
       end
 
-      def host_eager_name
+      def host_attribute_eager_name
         raise NotImplementedError
       end
 
@@ -46,23 +47,33 @@ def extract_disk_gb(param)
 
       private
 
+      def load_hosts_eagerly(missing_hosts_resources, host_objects, eager_attribute)
+        relevant_hosts = Host::Managed.where(name: missing_hosts_resources.keys).includes(eager_attribute)
+        relevant_hosts = relevant_hosts.compact
+        if relevant_hosts.size < missing_hosts_resources.size # Add non-eagerly loaded host objects
+          relevant_hosts_names = relevant_hosts.map(&:name)
+          (missing_hosts_resources.keys - relevant_hosts_names).each do |missing_host_name|
+            relevant_hosts << host_objects[missing_host_name]
+          end
+        end
+        relevant_hosts
+      end
+
       def collect_attribute_from_hosts(host_list, attribute_name)
-        host_values = []
+        host_values = {}
         host_list.each do |host|
-          host_attribute = host.send(attribute_name)
-          host_values << [host.id, host_attribute] if host_attribute.present?
+          attribute_value = host.send(attribute_name)
+          host_values[host.name] = attribute_value if attribute_value.present?
         rescue StandardError
           # skip hosts whose attribute couldn't be collected
         end
         host_values
       end
 
-      def process_resource!(resources_sum, missing_res_per_host, resource_name, host_id, attribute_content)
+      def process_resource(resource_name, attribute_content)
         resource_value = method(RESOURCE_FUNCTION_MAP[resource_name]).call(attribute_content)
-        return unless resource_value
-
-        resources_sum[resource_name] += resource_value
-        missing_res_per_host[host_id].delete(resource_name)
+        return nil unless resource_value
+        resource_value
       end
     end
   end