Merge pull request #22238 from agrare/morphy_class_for_ems

[MORPHY] Class for ems

app/models/cloud_network.rb

@@ -47,8 +47,7 @@ class CloudNetwork < ApplicationRecord
   virtual_total :total_vms, :vms
 
   def self.class_by_ems(ext_management_system, _external = false)
-    # TODO: A factory on ExtManagementSystem to return class for each provider
-    ext_management_system && ext_management_system.class::CloudNetwork
+    ext_management_system&.class_by_ems(:CloudNetwork)
   end
 
   def self.tenant_id_clause_format(tenant_ids)

app/models/cloud_subnet.rb

@@ -51,8 +51,7 @@ def dns_nameservers_show
   virtual_total :total_vms, :vms, :uses => :vms
 
   def self.class_by_ems(ext_management_system)
-    # TODO: use a factory on ExtManagementSystem side to return correct class for each provider
-    ext_management_system && ext_management_system.class::CloudSubnet
+    ext_management_system&.class_by_ems(:CloudSubnet)
   end
 
   def delete_cloud_subnet

app/models/cloud_tenant.rb

@@ -39,14 +39,14 @@ class CloudTenant < ApplicationRecord
   virtual_total :total_vms, :vms
 
   def self.class_by_ems(ext_management_system)
-    ext_management_system && ext_management_system.class::CloudTenant
+    ext_management_system&.class_by_ems(:CloudTenant)
   end
 
   def self.create_cloud_tenant(ems_id, options = {})
     ext_management_system = ExtManagementSystem.find_by(:id => ems_id)
     raise ArgumentError, _("ext_management_system cannot be nil") if ext_management_system.nil?
 
-    klass = class_by_ems(ext_management_system)
+    klass = ext_management_system.class_by_ems(:CloudTenant)
     klass.raw_create_cloud_tenant(ext_management_system, options)
   end
 
@@ -66,7 +66,7 @@ def self.create_cloud_tenant_queue(userid, ext_management_system, options = {})
     }
 
     queue_opts = {
-      :class_name  => class_by_ems(ext_management_system).name,
+      :class_name  => ext_management_system.class_by_ems(:CloudTenant).name,
       :method_name => 'create_cloud_tenant',
       :priority    => MiqQueue::HIGH_PRIORITY,
       :role        => 'ems_operations',

app/models/cloud_volume.rb

@@ -51,9 +51,7 @@ def self.available
   end
 
   def self.class_by_ems(ext_management_system)
-    # TODO(lsmola) taken from OrchesTration stacks, correct approach should be to have a factory on ExtManagementSystem
-    # side, that would return correct class for each provider
-    ext_management_system && ext_management_system.class::CloudVolume
+    ext_management_system&.class_by_ems(:CloudVolume)
   end
 
   def self.my_zone(ems)
@@ -93,7 +91,7 @@ def self.create_volume(ems_id, options = {})
     ext_management_system = ExtManagementSystem.find(ems_id)
     raise ArgumentError, _("ext_management_system cannot be found") if ext_management_system.nil?
 
-    klass = class_by_ems(ext_management_system)
+    klass = ext_management_system.class_by_ems(:CloudVolume)
     klass.raw_create_volume(ext_management_system, options)
   end
 

app/models/cloud_volume_snapshot.rb

@@ -14,7 +14,7 @@ class CloudVolumeSnapshot < ApplicationRecord
   virtual_total :total_based_volumes, :based_volumes
 
   def self.class_by_ems(ext_management_system)
-    ext_management_system && ext_management_system.class::CloudVolumeSnapshot
+    ext_management_system&.class_by_ems(:CloudVolumeSnapshot)
   end
 
   def self.my_zone(ems)

app/models/cloud_volume_type.rb

@@ -10,7 +10,7 @@ class CloudVolumeType < ApplicationRecord
   belongs_to :ext_management_system, :foreign_key => :ems_id, :class_name => "ExtManagementSystem"
 
   def self.class_by_ems(ext_management_system)
-    ext_management_system && ext_management_system.class::CloudVolumeType
+    ext_management_system&.class_by_ems(:CloudVolumeType)
   end
 
   def self.display_name(number = 1)

app/models/ext_management_system.rb

@@ -1033,21 +1033,18 @@ def self.class_for_ems(class_name, inherit = false)
     nil
   end
 
-  # Takes a child class (e.g.: "NetworkRouter") and returns the EMS specific version of it
+  # Factory that takes a child class (e.g.: "NetworkRouter")
+  #         and returns the EMS specific version of it
+  # This can be overridden case by case in EMS specific implementations
   #
   # @param [String|Symbol] class_name name of the child class
-  # @param [Boolean] inherit - return the base class if the child does not have a specific one (default: false - return nil if no child)
   # @returns The EMS specific version of the class requested
-  def class_for_ems(class_name, inherit = false)
-    self.class.class_for_ems(class_name, inherit)
-  end
-
-  def self.class_for_ems(class_name, inherit = false)
-    class_name = class_name.name if class_name.kind_of?(Class)
-    const_get(class_name, inherit)
+  def self.class_by_ems(class_name)
+    const_get(class_name, false)
   rescue NameError
     nil
   end
+  delegate :class_by_ems, :to => :class
 
   # @return [Boolean] true if a datastore exists for this type of ems
   def self.datastore?

app/models/flavor.rb

@@ -37,7 +37,7 @@ def name_with_details
   end
 
   def self.class_by_ems(ext_management_system)
-    ext_management_system.class::Flavor
+    ext_management_system&.class_by_ems(:Flavor)
   end
 
   def self.tenant_joins_clause(scope)
@@ -79,8 +79,7 @@ def self.create_flavor(ems_id, options)
     raise ArgumentError, _("ems cannot be nil") if ems_id.nil?
     ext_management_system = ExtManagementSystem.find(ems_id)
     raise ArgumentError, _("ems cannot be found") if ext_management_system.nil?
-
-    klass = class_by_ems(ext_management_system)
+    klass = ext_management_system.class_by_ems(:Flavor)
     klass.raw_create_flavor(ext_management_system, options)
   end
 

app/models/floating_ip.rb

@@ -22,8 +22,7 @@ def self.available
   end
 
   def self.class_by_ems(ext_management_system)
-    # TODO: use a factory on ExtManagementSystem side to return correct class for each provider
-    ext_management_system && ext_management_system.class::FloatingIp
+    ext_management_system&.class_by_ems(:FloatingIp)
   end
 
   def self.display_name(number = 1)

app/models/host_initiator.rb

@@ -23,9 +23,7 @@ def my_zone
   end
 
   def self.class_by_ems(ext_management_system)
-    # TODO(lsmola) taken from Orchestration stacks, correct approach should be to have a factory on ExtManagementSystem
-    # side, that would return correct class for each provider
-    ext_management_system && ext_management_system.class::HostInitiator
+    ext_management_system&.class_by_ems(:HostInitiator)
   end
 
   def refresh_ems
@@ -66,7 +64,7 @@ def self.create_host_initiator(ems_id, options = {})
     ext_management_system = ExtManagementSystem.find_by(:id => ems_id)
     raise ArgumentError, _("ext_management_system cannot be found") if ext_management_system.nil?
 
-    klass = class_by_ems(ext_management_system)
+    klass = ext_management_system.class_by_ems(:HostInitiator)
     klass.raw_create_host_initiator(ext_management_system, options)
   end
 

app/models/manageiq/providers/cloud_manager/auth_key_pair.rb

@@ -7,7 +7,7 @@ class ManageIQ::Providers::CloudManager::AuthKeyPair < ::Authentication
   include_concern 'Operations'
 
   def self.class_by_ems(ext_management_system)
-    ext_management_system.class::AuthKeyPair
+    ext_management_system&.class_by_ems(:AuthKeyPair)
   end
 
   # Create an auth key pair as a queued task and return the task id. The queue
@@ -38,7 +38,7 @@ def self.create_key_pair(ems_id, options)
     ext_management_system = ExtManagementSystem.find(ems_id)
     raise ArgumentError, _("ems cannot be found") if ext_management_system.nil?
 
-    klass = class_by_ems(ext_management_system)
+    klass = ext_management_system.class_by_ems(:AuthKeyPair)
     # TODO(maufart): add cloud_tenant to database table?
     created_key_pair = klass.raw_create_key_pair(ext_management_system, options)
     klass.create(

app/models/manageiq/providers/cloud_manager/template.rb

@@ -12,7 +12,7 @@ def snapshot?
   end
 
   def self.class_by_ems(ext_management_system)
-    ext_management_system.class::Template
+    ext_management_system&.class_by_ems(:Template)
   end
 
   # Create a cloud image as a queued task and return the task id. The queue
@@ -51,7 +51,7 @@ def self.create_image(ems_id, options)
     ext_management_system = ExtManagementSystem.find(ems_id)
     raise ArgumentError, _("ems cannot be found") if ext_management_system.nil?
 
-    klass = class_by_ems(ext_management_system)
+    klass = ext_management_system.class_by_ems(:Template)
     klass.raw_create_image(ext_management_system, options)
   end
 

app/models/network_router.rb

@@ -49,8 +49,7 @@ class NetworkRouter < ApplicationRecord
   virtual_total :total_vms, :vms
 
   def self.class_by_ems(ext_management_system)
-    # TODO: use a factory on ExtManagementSystem side to return correct class for each provider
-    ext_management_system && ext_management_system.class::NetworkRouter
+    ext_management_system&.class_by_ems(:NetworkRouter)
   end
 
   private

app/models/network_service.rb

@@ -20,8 +20,7 @@ class NetworkService < ApplicationRecord
   virtual_total :security_policy_rules_count, :security_policy_rules
 
   def self.class_by_ems(ext_management_system)
-    # TODO: use a factory on ExtManagementSystem side to return correct class for each provider
-    ext_management_system && ext_management_system.class::NetworkService
+    ext_management_system&.class_by_ems(:NetworkService)
   end
 
   def self.display_name(number = 1)

app/models/network_service_entry.rb

@@ -12,7 +12,6 @@ class NetworkServiceEntry < ApplicationRecord
   has_many :network_service_entries, :foreign_key => :ems_id, :dependent => :destroy
 
   def self.class_by_ems(ext_management_system)
-    # TODO: use a factory on ExtManagementSystem side to return correct class for each provider
-    ext_management_system && ext_management_system.class::NetworkServiceEntry
+    ext_management_system&.class_by_ems(:NetworkServiceEntry)
   end
 end

app/models/physical_storage.rb

@@ -104,14 +104,12 @@ def self.create_physical_storage(ems_id, options = {})
     ext_management_system = ExtManagementSystem.find_by(:id => ems_id)
     raise ArgumentError, _("ext_management_system cannot be found") if ext_management_system.nil?
 
-    klass = class_by_ems(ext_management_system)
+    klass = ext_management_system.class_by_ems(:PhysicalStorage)
     klass.raw_create_physical_storage(ext_management_system, options)
   end
 
   def self.class_by_ems(ext_management_system)
-    # TODO(lsmola) taken from Orchestration stacks, correct approach should be to have a factory on ExtManagementSystem
-    # side, that would return correct class for each provider
-    ext_management_system && ext_management_system.class::PhysicalStorage
+    ext_management_system&.class_by_ems(:PhysicalStorage)
   end
 
   def self.raw_create_physical_storage(_ext_management_system, _options = {})

app/models/security_group.rb

@@ -36,8 +36,7 @@ def self.non_cloud_network
   end
 
   def self.class_by_ems(ext_management_system)
-    # TODO: use a factory on ExtManagementSystem side to return correct class for each provider
-    ext_management_system && ext_management_system.class::SecurityGroup
+    ext_management_system&.class_by_ems(:SecurityGroup)
   end
 
   def update_security_group_queue(userid, options = {})

app/models/security_policy.rb

@@ -16,8 +16,7 @@ class SecurityPolicy < ApplicationRecord
   virtual_total :rules_count, :rules
 
   def self.class_by_ems(ext_management_system)
-    # TODO: use a factory on ExtManagementSystem side to return correct class for each provider
-    ext_management_system && ext_management_system.class::SecurityPolicy
+    ext_management_system&.class_by_ems(:SecurityPolicy)
   end
 
   def update_security_policy_queue(userid, options = {})

app/models/security_policy_rule.rb

@@ -27,8 +27,7 @@ class SecurityPolicyRule < ApplicationRecord
   virtual_total :network_services_count, :network_services
 
   def self.class_by_ems(ext_management_system)
-    # TODO: use a factory on ExtManagementSystem side to return correct class for each provider
-    ext_management_system && ext_management_system.class::SecurityPolicyRule
+    ext_management_system&.class_by_ems(:SecurityPolicyRule)
   end
 
   def update_security_policy_rule_queue(userid, options = {})

app/models/storage_resource.rb

@@ -15,8 +15,6 @@ class StorageResource < ApplicationRecord
   acts_as_miq_taggable
 
   def self.class_by_ems(ext_management_system)
-    # TODO(lsmola) taken from Orchestration stacks, correct approach should be to have a factory on ExtManagementSystem
-    # side, that would return correct class for each provider
-    ext_management_system && ext_management_system.class::StorageResource
+    ext_management_system&.class_by_ems(:StorageResource)
   end
 end

app/models/storage_service.rb

@@ -13,8 +13,6 @@ class StorageService < ApplicationRecord
   acts_as_miq_taggable
 
   def self.class_by_ems(ext_management_system)
-    # TODO(lsmola) taken from Orchestration stacks, correct approach should be to have a factory on ExtManagementSystem
-    # side, that would return correct class for each provider
-    ext_management_system && ext_management_system.class::StorageService
+    ext_management_system&.class_by_ems(:StorageService)
   end
 end

app/models/volume_mapping.rb

@@ -22,9 +22,7 @@ def my_zone
   end
 
   def self.class_by_ems(ext_management_system)
-    # TODO(lsmola) taken from Orchestration stacks, correct approach should be to have a factory on ExtManagementSystem
-    # side, that would return correct class for each provider
-    ext_management_system && ext_management_system.class::VolumeMapping
+    ext_management_system&.class_by_ems(:VolumeMapping)
   end
 
   def raw_delete_volume_mapping
@@ -78,7 +76,7 @@ def self.create_volume_mapping(ems_id, options = {})
     ext_management_system = ExtManagementSystem.find_by(:id => ems_id)
     raise ArgumentError, _("ext_management_system cannot be found") if ext_management_system.nil?
 
-    klass = class_by_ems(ext_management_system)
+    klass = ext_management_system.class_by_ems(:VolumeMapping)
     klass.raw_create_volume_mapping(ext_management_system, options)
   end
 

spec/models/ext_management_system_spec.rb

@@ -659,41 +659,27 @@
     end
   end
 
-  describe "#class_for_ems" do
+  describe "#class_by_ems" do
     it 'returns a concrete target subclass for a concrete EMS' do
       ems = FactoryBot.create(:ems_openstack)
 
-      expect(ems.class.class_for_ems("Vm", true)).to eq(ems.class::Vm)
-      expect(ems.class_for_ems("Vm", true)).to eq(ems.class::Vm)
-      expect(ems.class_for_ems("Vm")).to eq(ems.class::Vm)
-      expect(ems.class_for_ems(:Vm)).to eq(ems.class::Vm)
-      expect(ems.class_for_ems(::Vm)).to eq(ems.class::Vm)
-    end
-
-    # this is documenting what NOT to do
-    # it is showing that the results are sporadic
-    it 'expects generic target subclass (not a concrete one)' do
-      ems = FactoryBot.create(:ems_openstack)
-      expect(ems.class_for_ems(ems.class::Vm, true)).to eq(ems.class::Vm)
-      expect(ems.class_for_ems(ems.class::Vm)).to be nil
+      expect(ems.class_by_ems("Vm")).to eq(ems.class::Vm)
+      expect(ems.class_by_ems(:Vm)).to eq(ems.class::Vm)
     end
 
     it 'returns nil for unknown class' do
       ems = FactoryBot.create(:ext_management_system)
-      expect(ems.class_for_ems("RandomSymbol", true)).to be nil
-      expect(ems.class_for_ems(:RandomSymbol)).to be nil
+      expect(ems.class_by_ems(:RandomSymbol)).to be nil
     end
 
     it 'returns a concrete target subclass for a concrete EMS subclass' do
       ems = FactoryBot.create(:ems_openstack_network)
-      expect(ems.class_for_ems("NetworkRouter", true)).to eq(ems.class::NetworkRouter)
-      expect(ems.class_for_ems("NetworkRouter")).to eq(ems.class::NetworkRouter)
+      expect(ems.class_by_ems("NetworkRouter")).to eq(ems.class::NetworkRouter)
     end
 
     it 'returns the base target class when the EMS does not have a subclass' do
       ems = FactoryBot.create(:ems_infra)
-      expect(ems.class_for_ems("NetworkRouter", true)).to eq(::NetworkRouter)
-      expect(ems.class_for_ems("NetworkRouter")).to eq(nil)
+      expect(ems.class_by_ems("NetworkRouter")).to eq(nil)
     end
   end
 

spec/models/flavor_spec.rb

@@ -22,7 +22,7 @@
   context 'when calling create_flavor method' do
     it 'should call raw_create_flavor' do
       flavor_double = class_double('ManageIQ::Providers::Openstack::CloudManager::Flavor')
-      allow(subject.class).to receive(:class_by_ems).and_return(flavor_double)
+      allow(ems.class).to receive(:class_by_ems).and_return(flavor_double)
       expect(flavor_double).to receive(:raw_create_flavor)
       subject.class.create_flavor(ems.id, {})
     end