Use foreign_key on the base model's name if ems_events association do…

…esn't exist

EmsCluster#ems_event_filter_column was correctly ems_cluster_id,
subclasses had cluster_id as the column.

Fix PhysicalSwitch to correctly implement ems_events association. Previously,
it assumed event_streams association was the same as ems_events just because
all event_streams for that model have always been ems events.  Instead, we
implement event_streams association so we can correctly detect the foreign
key in ems_event_filter_column.

Fix tests to test descendant classes so we can recreate and verify the fix for EmsCluster
base class and subclasses.  Add HostAggregate and hysicalServerProfile to pending list
as they include the mixin but don't implement the interface (event_where_clause, _id column, no
ems_events association).

app/models/ems_cluster.rb

@@ -204,6 +204,12 @@ def event_where_clause(assoc = :ems_events)
     cond_params
   end
 
+  # TODO: ems_events already exists as a method but it uses
+  # event_where_clause which tacks on additional possibly expensive queries
+  # such as all events for all hosts or vms in the cluster.
+  # Should we move to looking at ems events specifically for the ems cluster?
+  # like this:
+  #   has_many :ems_events, :foreign_key => :ems_cluster_id
   def ems_events
     ewc = event_where_clause
     return [] if ewc.blank?

app/models/host_aggregate.rb

@@ -2,6 +2,9 @@ class HostAggregate < ApplicationRecord
   include SupportsFeatureMixin
   include NewWithTypeStiMixin
   include Metric::CiMixin
+
+  # TODO: this model doesn't have an event_where_clause, doesn't have a *_id in event_streams but includes
+  # event mixin.  Track down if we need timeline events for this as they look to not be completely supported yet.
   include EventMixin
   include ProviderObjectMixin
 

app/models/mixins/event_mixin.rb

@@ -63,7 +63,7 @@ def find_one_event(assoc, order)
 
   module ClassMethods
     def ems_event_filter_column
-      @ems_event_filter_column ||= reflect_on_association(:ems_events).try(:foreign_key) || name.foreign_key
+      @ems_event_filter_column ||= reflect_on_association(:ems_events).try(:foreign_key) || base_model.name.foreign_key
     end
 
     def miq_event_filter_column

app/models/physical_server_profile.rb

@@ -4,6 +4,9 @@ class PhysicalServerProfile < ApplicationRecord
   include NewWithTypeStiMixin
   include TenantIdentityMixin
   include SupportsFeatureMixin
+
+  # TODO: this model doesn't have an event_where_clause, doesn't have a *_id in event_streams but includes
+  # event mixin.  Track down if we need timeline events for this as they look to not be completely supported yet.
   include EventMixin
   include ProviderObjectMixin
   include EmsRefreshMixin

app/models/physical_switch.rb

@@ -9,7 +9,11 @@ class PhysicalSwitch < Switch
   has_one :asset_detail, :as => :resource, :dependent => :destroy, :inverse_of => :resource
   has_one :hardware, :dependent => :destroy, :foreign_key => :switch_id, :inverse_of => :physical_switch
   has_many :physical_network_ports, :dependent => :destroy, :foreign_key => :switch_id
+
+  # TODO: Deprecate event_streams if it makes sense, find callers, and change to use ems_events.  Even though event_streams
+  # have only ever been ems_events in this model, we shouldn't rely on that, so callers should use ems_events.
   has_many :event_streams, :inverse_of => :physical_switch, :dependent => :nullify
+  has_many :ems_events, :inverse_of => :physical_switch, :dependent => :nullify
 
   has_many :connected_components, :through => :physical_network_ports, :source => :connected_computer_system
 

spec/models/mixins/event_mixin_spec.rb

@@ -46,32 +46,51 @@ def event_where_clause(assoc)
     # 2) EmsCluster#event_where_clause does an OR:  ems_cluster_id OR (host_id in ? OR dest_host_id IN ?) OR (vm_or_template in ? OR dest_vm_or_template_id in ?) (for now we just do ems_cluster_id in ems_event_filter)
     # 3) Host#event_where_clause does an OR: host_id OR dest_host_id... right now we only do host_id in ems_event_filter
     # 4) VmOrTemplate#event_where_clause does an OR: vm_or_template_id OR dest_vm_or_template_id, we only do vm_or_template_id in ems_event_filter
+    # 5) The following are tested below in the custom behavior section because they have different setup:
+      # Container           container_id
+      # ContainerGroup      container_group_id
+      # ContainerNode       container_node_id
+      # ContainerProject    container_project_id
+      # ContainerReplicator container_replicator_id
+    # The following classes are missing the event_where_clause and their _id column isn't in event_streams table... fix this by removing event mixin or making it work by adding the column.
+    not_implemented = %w[HostAggregate PhysicalServerProfile]
     %w[
       AvailabilityZone    availability_zone_id
       EmsCluster          ems_cluster_id
       ExtManagementSystem ems_id
       Host                host_id
+      HostAggregate       host_aggregate_id
       PhysicalChassis     physical_chassis_id
       PhysicalServer      physical_server_id
+      PhysicalServerProfile physical_server_profile_id
       PhysicalStorage     physical_storage_id
       PhysicalSwitch      physical_switch_id
       VmOrTemplate        vm_or_template_id
       Vm                  vm_or_template_id
     ].each_slice(2) do |klass, column|
-      it "#{klass} uses #{column} and target_id and target_type" do
-        obj = FactoryBot.create(klass.tableize.singularize)
-        expect(obj.event_stream_filters["EmsEvent"]).to eq(column => obj.id)
-        expect(obj.event_stream_filters.dig("MiqEvent", "target_id")).to eq(obj.id)
-        expect(obj.event_stream_filters.dig("MiqEvent", "target_type")).to eq(obj.class.base_class.name)
-      end
-
-      it "#{klass} behaves like event_where_clause for ems_events" do
-        obj = FactoryBot.create(klass.tableize.singularize)
-        event = FactoryBot.create(:event_stream, column => obj.id)
-        FactoryBot.create(:event_stream)
-        expect(EventStream.where(obj.event_stream_filters["EmsEvent"]).to_a).to eq([event])
-        expect(EventStream.where(obj.event_where_clause(:ems_events)).to_a).to eq([event])
+      skip = true if not_implemented.include?(klass)
+      klasses = klass.constantize.descendants.collect(&:name).unshift(klass)
+      klasses.each do |klass|
+        it "#{klass} uses #{column} and target_id and target_type" do
+          skip("Class: #{klass} hasn't fully implemented timeline events") if skip
+          begin
+            obj = FactoryBot.build(klass.tableize.singularize, :name => "test")
+          rescue NameError
+            skip "Unable to build factory from name: #{klass}, possibly due to inflections"
+          end
+          expect(obj.event_stream_filters["EmsEvent"]).to eq(column => obj.id)
+          expect(obj.event_stream_filters.dig("MiqEvent", "target_id")).to eq(obj.id)
+          expect(obj.event_stream_filters.dig("MiqEvent", "target_type")).to eq(obj.class.base_class.name)
+        end
       end
+    it "#{klass} behaves like event_where_clause for ems_events" do
+      skip("Class: #{klass} hasn't implemented timeline events.  This could be due to no event_where_clause, ems_events association, and/or _id column in event_streams") if skip
+      obj = FactoryBot.create(klass.tableize.singularize)
+      event = FactoryBot.create(:event_stream, column => obj.id)
+      FactoryBot.create(:event_stream)
+      expect(EventStream.where(obj.event_stream_filters["EmsEvent"]).to_a).to eq([event])
+      expect(EventStream.where(obj.event_where_clause(:ems_events)).to_a).to eq([event])
+    end
 
       # # TODO: some classes don't have this implemented or don't have columns for this
       # # Do we consolidate policy events and miq events?