From 0868e07d517da825c17e133758cc8b96b7398d4c Mon Sep 17 00:00:00 2001
From: Fabien Dupont <fdupont@redhat.com>
Date: Fri, 26 Oct 2018 10:05:33 -0400
Subject: [PATCH 1/4] Add a validation for conversion hosts

---
 ...ce_template_transformation_plan_request.rb | 10 +++++
 ...mplate_transformation_plan_request_spec.rb | 45 +++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/app/models/service_template_transformation_plan_request.rb b/app/models/service_template_transformation_plan_request.rb
index 8a3c38c085a..468af43aee2 100644
--- a/app/models/service_template_transformation_plan_request.rb
+++ b/app/models/service_template_transformation_plan_request.rb
@@ -15,6 +15,16 @@ def source_vms
     vm_resources.where(:status => [ServiceResource::STATUS_QUEUED, ServiceResource::STATUS_FAILED]).pluck(:resource_id)
   end
 
+  def validate_conversion_hosts
+    transformation_mapping.transformation_mapping_items.each do |item|
+      next unless %w(EmsCluster CloudTenant).include?(item.source_type)
+      ems = item.destination_type.constantize.find(item.destination_id).ext_management_system
+      conversion_hosts = ConversionHost.all.select { |ch| ch.ext_management_system == ems }
+      return false if conversion_hosts.empty?
+    end
+    true
+  end
+
   def validate_vm(_vm_id)
     # TODO: enhance the logic to determine whether this VM can be included in this request
     true
diff --git a/spec/models/service_template_transformation_plan_request_spec.rb b/spec/models/service_template_transformation_plan_request_spec.rb
index d9c718958dd..8f1c595216d 100644
--- a/spec/models/service_template_transformation_plan_request_spec.rb
+++ b/spec/models/service_template_transformation_plan_request_spec.rb
@@ -5,6 +5,32 @@
       ServiceResource.new(:resource => vm, :status => status)
     end
   end
+
+  let(:dst_ems) { FactoryGirl.create(:ext_management_system) }
+  let(:src_cluster) { FactoryGirl.create(:ems_cluster) }
+  let(:dst_cluster) { FactoryGirl.create(:ems_cluster, :ext_management_system => dst_ems) }
+
+  let(:mapping) do
+    FactoryGirl.create(
+      :transformation_mapping,
+      :transformation_mapping_items => [TransformationMappingItem.new(:source => src_cluster, :destination => dst_cluster)]
+    )   
+  end 
+
+  let(:catalog_item_options) do
+    {   
+      :name        => 'Transformation Plan',
+      :description => 'a description',
+      :config_info => {
+        :transformation_mapping_id => mapping.id,
+        :actions                   => [
+          {:vm_id => vms.first.id.to_s, :pre_service => false, :post_service => false},
+          {:vm_id => vms.last.id.to_s, :pre_service => false, :post_service => false},
+        ],
+      }
+    }   
+  end 
+
   let(:plan) { FactoryGirl.create(:service_template_transformation_plan, :service_resources => vm_requests) }
   let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }
 
@@ -28,6 +54,25 @@
     end
   end
 
+  describe '#validate_conversion_hosts' do
+    let(:plan) { ServiceTemplateTransformationPlan.create_catalog_item(catalog_item_options) }
+    let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }
+
+    context 'no conversion host exists in EMS' do
+      let(:host) { FactoryGirl.create(:host, :ext_management_system => FactoryGirl.create(:ext_management_system, :zone => FactoryGirl.create(:zone))) }
+      let(:conversion_host) { FactoryGirl.create(:conversion_host, :resource => host) }
+
+      it { expect(request.validate_conversion_hosts).to eq(false) }
+    end
+
+    context 'conversion host exists in EMS' do
+      let(:host) { FactoryGirl.create(:host, :ext_management_system => dst_ems) }
+      let(:conversion_host) { FactoryGirl.create(:conversion_host, :resource => host) }
+
+      it { expect(request.validate_conversion_hosts).to eq(false) }
+    end
+  end
+
   describe '#validate_vm' do
     it { expect(request.validate_vm(vms[0].id)).to be_truthy }
   end

From ac22e42845a26ad0f2755f177deebf852d849e9b Mon Sep 17 00:00:00 2001
From: Fabien Dupont <fdupont@redhat.com>
Date: Fri, 26 Oct 2018 10:27:00 -0400
Subject: [PATCH 2/4] Fix rubocop

---
 ...ervice_template_transformation_plan_request_spec.rb | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/spec/models/service_template_transformation_plan_request_spec.rb b/spec/models/service_template_transformation_plan_request_spec.rb
index 8f1c595216d..02cbb33eb16 100644
--- a/spec/models/service_template_transformation_plan_request_spec.rb
+++ b/spec/models/service_template_transformation_plan_request_spec.rb
@@ -14,11 +14,11 @@
     FactoryGirl.create(
       :transformation_mapping,
       :transformation_mapping_items => [TransformationMappingItem.new(:source => src_cluster, :destination => dst_cluster)]
-    )   
-  end 
+    )
+  end
 
   let(:catalog_item_options) do
-    {   
+    {
       :name        => 'Transformation Plan',
       :description => 'a description',
       :config_info => {
@@ -28,8 +28,8 @@
           {:vm_id => vms.last.id.to_s, :pre_service => false, :post_service => false},
         ],
       }
-    }   
-  end 
+    }
+  end
 
   let(:plan) { FactoryGirl.create(:service_template_transformation_plan, :service_resources => vm_requests) }
   let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }

From 8342dbbab61d84b0d4af8ed0e877b6bdfffa6214 Mon Sep 17 00:00:00 2001
From: Fabien Dupont <fdupont@redhat.com>
Date: Tue, 30 Oct 2018 10:13:59 -0400
Subject: [PATCH 3/4] Add associations to ext_management_system

---
 app/models/ext_management_system.rb                        | 7 +++++++
 app/models/service_template_transformation_plan_request.rb | 7 ++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb
index 08d4a0f43d4..91315dd8c8f 100644
--- a/app/models/ext_management_system.rb
+++ b/app/models/ext_management_system.rb
@@ -84,6 +84,9 @@ def self.api_allowed_attributes
   has_many :service_offerings, :foreign_key => :ems_id, :dependent => :destroy, :inverse_of => :ext_management_system
   has_many :service_parameters_sets, :foreign_key => :ems_id, :dependent => :destroy, :inverse_of => :ext_management_system
 
+  has_many :host_conversion_hosts, :through => :hosts, :source => :conversion_host
+  has_many :vm_conversion_hosts, :through => :vms, :source => :conversion_host
+
   validates :name,     :presence => true, :uniqueness => {:scope => [:tenant_id]}
   validates :hostname, :presence => true, :if => :hostname_required?
   validate :hostname_uniqueness_valid?, :hostname_format_valid?, :if => :hostname_required?
@@ -611,6 +614,10 @@ def vm_log_user_event(_vm, user_event)
     $log.warn("User event logging is not available on [#{self.class.name}] Name:[#{name}]")
   end
 
+  def conversion_hosts
+    host_conversion_hosts + vm_conversion_hosts
+  end
+
   #
   # Metric methods
   #
diff --git a/app/models/service_template_transformation_plan_request.rb b/app/models/service_template_transformation_plan_request.rb
index 468af43aee2..769d4cab31b 100644
--- a/app/models/service_template_transformation_plan_request.rb
+++ b/app/models/service_template_transformation_plan_request.rb
@@ -16,11 +16,8 @@ def source_vms
   end
 
   def validate_conversion_hosts
-    transformation_mapping.transformation_mapping_items.each do |item|
-      next unless %w(EmsCluster CloudTenant).include?(item.source_type)
-      ems = item.destination_type.constantize.find(item.destination_id).ext_management_system
-      conversion_hosts = ConversionHost.all.select { |ch| ch.ext_management_system == ems }
-      return false if conversion_hosts.empty?
+    transformation_mapping.transformation_mapping_items.select { |item| %w(EmsCluster CloudTenant).include?(item.source_type) }.each do |item|
+      return false if item.destination.ext_management_system.conversion_hosts.empty?
     end
     true
   end

From d7273e1049002773846c66fb859bbf29a529b871 Mon Sep 17 00:00:00 2001
From: Fabien Dupont <fdupont@redhat.com>
Date: Wed, 31 Oct 2018 06:19:33 -0400
Subject: [PATCH 4/4] Simplify method. Add test for VM conversion host.

---
 ...ce_template_transformation_plan_request.rb |   7 +-
 ...mplate_transformation_plan_request_spec.rb | 138 +++++++++++++-----
 2 files changed, 106 insertions(+), 39 deletions(-)

diff --git a/app/models/service_template_transformation_plan_request.rb b/app/models/service_template_transformation_plan_request.rb
index 769d4cab31b..73b26083696 100644
--- a/app/models/service_template_transformation_plan_request.rb
+++ b/app/models/service_template_transformation_plan_request.rb
@@ -16,10 +16,11 @@ def source_vms
   end
 
   def validate_conversion_hosts
-    transformation_mapping.transformation_mapping_items.select { |item| %w(EmsCluster CloudTenant).include?(item.source_type) }.each do |item|
-      return false if item.destination.ext_management_system.conversion_hosts.empty?
+    transformation_mapping.transformation_mapping_items.select do |item|
+      %w(EmsCluster CloudTenant).include?(item.source_type)
+    end.all? do |item|
+      item.destination.ext_management_system.conversion_hosts.present?
     end
-    true
   end
 
   def validate_vm(_vm_id)
diff --git a/spec/models/service_template_transformation_plan_request_spec.rb b/spec/models/service_template_transformation_plan_request_spec.rb
index 02cbb33eb16..f611017466d 100644
--- a/spec/models/service_template_transformation_plan_request_spec.rb
+++ b/spec/models/service_template_transformation_plan_request_spec.rb
@@ -6,31 +6,6 @@
     end
   end
 
-  let(:dst_ems) { FactoryGirl.create(:ext_management_system) }
-  let(:src_cluster) { FactoryGirl.create(:ems_cluster) }
-  let(:dst_cluster) { FactoryGirl.create(:ems_cluster, :ext_management_system => dst_ems) }
-
-  let(:mapping) do
-    FactoryGirl.create(
-      :transformation_mapping,
-      :transformation_mapping_items => [TransformationMappingItem.new(:source => src_cluster, :destination => dst_cluster)]
-    )
-  end
-
-  let(:catalog_item_options) do
-    {
-      :name        => 'Transformation Plan',
-      :description => 'a description',
-      :config_info => {
-        :transformation_mapping_id => mapping.id,
-        :actions                   => [
-          {:vm_id => vms.first.id.to_s, :pre_service => false, :post_service => false},
-          {:vm_id => vms.last.id.to_s, :pre_service => false, :post_service => false},
-        ],
-      }
-    }
-  end
-
   let(:plan) { FactoryGirl.create(:service_template_transformation_plan, :service_resources => vm_requests) }
   let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }
 
@@ -55,21 +30,112 @@
   end
 
   describe '#validate_conversion_hosts' do
-    let(:plan) { ServiceTemplateTransformationPlan.create_catalog_item(catalog_item_options) }
-    let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }
-
     context 'no conversion host exists in EMS' do
-      let(:host) { FactoryGirl.create(:host, :ext_management_system => FactoryGirl.create(:ext_management_system, :zone => FactoryGirl.create(:zone))) }
-      let(:conversion_host) { FactoryGirl.create(:conversion_host, :resource => host) }
-
-      it { expect(request.validate_conversion_hosts).to eq(false) }
+      let(:dst_ems) { FactoryGirl.create(:ext_management_system) }
+      let(:src_cluster) { FactoryGirl.create(:ems_cluster) }
+      let(:dst_cluster) { FactoryGirl.create(:ems_cluster, :ext_management_system => dst_ems) }
+
+      let(:mapping) do
+        FactoryGirl.create(
+          :transformation_mapping,
+          :transformation_mapping_items => [TransformationMappingItem.new(:source => src_cluster, :destination => dst_cluster)]
+        )
+      end
+
+      let(:catalog_item_options) do
+        {
+          :name        => 'Transformation Plan',
+          :description => 'a description',
+          :config_info => {
+            :transformation_mapping_id => mapping.id,
+            :actions                   => [
+              {:vm_id => vms.first.id.to_s, :pre_service => false, :post_service => false},
+              {:vm_id => vms.last.id.to_s, :pre_service => false, :post_service => false},
+            ],
+          }
+        }
+      end
+
+      let(:plan) { ServiceTemplateTransformationPlan.create_catalog_item(catalog_item_options) }
+      let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }
+
+      it 'returns false' do
+        host = FactoryGirl.create(:host, :ext_management_system => FactoryGirl.create(:ext_management_system, :zone => FactoryGirl.create(:zone)))
+        conversion_host = FactoryGirl.create(:conversion_host, :resource => host)
+        expect(request.validate_conversion_hosts).to be false
+      end
     end
 
-    context 'conversion host exists in EMS' do
-      let(:host) { FactoryGirl.create(:host, :ext_management_system => dst_ems) }
-      let(:conversion_host) { FactoryGirl.create(:conversion_host, :resource => host) }
+    context 'conversion host exists in EMS and resource is a Host' do
+      let(:dst_ems) { FactoryGirl.create(:ems_redhat) }
+      let(:src_cluster) { FactoryGirl.create(:ems_cluster) }
+      let(:dst_cluster) { FactoryGirl.create(:ems_cluster, :ext_management_system => dst_ems) }
+
+      let(:mapping) do
+        FactoryGirl.create(
+          :transformation_mapping,
+          :transformation_mapping_items => [TransformationMappingItem.new(:source => src_cluster, :destination => dst_cluster)]
+        )
+      end
+
+      let(:catalog_item_options) do
+        {
+          :name        => 'Transformation Plan',
+          :description => 'a description',
+          :config_info => {
+            :transformation_mapping_id => mapping.id,
+            :actions                   => [
+              {:vm_id => vms.first.id.to_s, :pre_service => false, :post_service => false},
+              {:vm_id => vms.last.id.to_s, :pre_service => false, :post_service => false},
+            ],
+          }
+        }
+      end
+
+      let(:plan) { ServiceTemplateTransformationPlan.create_catalog_item(catalog_item_options) }
+      let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }
+
+      it 'returns true' do
+        host = FactoryGirl.create(:host, :ext_management_system => dst_ems, :ems_cluster => dst_cluster)
+        conversion_host = FactoryGirl.create(:conversion_host, :resource => host)
+        expect(request.validate_conversion_hosts).to be true
+      end
+    end
 
-      it { expect(request.validate_conversion_hosts).to eq(false) }
+    context 'conversion host exists in EMS and resource is a Vm' do
+      let(:dst_ems) { FactoryGirl.create(:ems_openstack) }
+      let(:src_cluster) { FactoryGirl.create(:ems_cluster) }
+      let(:dst_cloud_tenant) { FactoryGirl.create(:cloud_tenant, :ext_management_system => dst_ems) }
+
+      let(:mapping) do
+        FactoryGirl.create(
+          :transformation_mapping,
+          :transformation_mapping_items => [TransformationMappingItem.new(:source => src_cluster, :destination => dst_cloud_tenant)]
+        )
+      end
+
+      let(:catalog_item_options) do
+        {
+          :name        => 'Transformation Plan',
+          :description => 'a description',
+          :config_info => {
+            :transformation_mapping_id => mapping.id,
+            :actions                   => [
+              {:vm_id => vms.first.id.to_s, :pre_service => false, :post_service => false},
+              {:vm_id => vms.last.id.to_s, :pre_service => false, :post_service => false},
+            ],
+          }
+        }
+      end
+
+      let(:plan) { ServiceTemplateTransformationPlan.create_catalog_item(catalog_item_options) }
+      let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }
+
+      it 'returns true' do
+        vm = FactoryGirl.create(:vm_openstack, :ext_management_system => dst_ems, :cloud_tenant => dst_cloud_tenant)
+        conversion_host = FactoryGirl.create(:conversion_host, :resource => vm)
+        expect(request.validate_conversion_hosts).to be true
+      end
     end
   end