Add cache columns to operating_systems table

NickLaMuro · NickLaMuro · commit a84a87ad1a84 · 2019-12-04T12:17:57.000-06:00
Both platform and image_name are relatively stable values that get
re-calculated every time they are accessed, and in the worst case,
causes it to traverse through many different sources to try and find a
proper `platform`/`image_name`.  This is compounded when you are trying
to fetch thousands of records at once.

This adds the columns to be cache store for those values, and should be
triggered on every update of those records, and related records (not
addressed in this patch).
diff --git a/db/migrate/20190108031411_add_image_name_to_operating_systems.rb b/db/migrate/20190108031411_add_image_name_to_operating_systems.rb
@@ -0,0 +1,159 @@
+class AddImageNameToOperatingSystems < ActiveRecord::Migration[5.0]
+  include MigrationHelper
+
+  StubOperatingSystemObj = Struct.new(:operating_system) do
+    attr_reader :hardware # always nil
+    def name
+      ""
+    end
+  end
+
+  class OperatingSystem < ActiveRecord::Base
+    belongs_to :host
+    belongs_to :vm_or_template
+    belongs_to :computer_system
+
+    # Using a `before_save` here since this is the mechanism that will be used
+    # in the app.  Causes a bit of issues in the specs, but proves that this
+    # would work moving forward.
+    before_save :update_platform_and_image_name
+
+    def update_platform_and_image_name
+      obj = case # rubocop:disable Style/EmptyCaseCondition
+            when host_id            then host
+            when vm_or_template_id  then vm_or_template
+            when computer_system_id then computer_system
+            else
+              StubOperatingSystemObj.new(self)
+            end
+
+      if obj
+        self.image_name = self.class.image_name(obj)
+        self.platform   = self.image_name.split("_").first # rubocop:disable Style/RedundantSelf
+      end
+    end
+
+    # rubocop:disable Style/PercentLiteralDelimiters
+    @@os_map = [ # rubocop:disable Style/ClassVars
+      ["windows_generic", %w(winnetenterprise w2k3 win2k3 server2003 winnetstandard servernt)],
+      ["windows_generic", %w(winxppro winxp)],
+      ["windows_generic", %w(vista longhorn)],
+      ["windows_generic", %w(win2k win2000)],
+      ["windows_generic", %w(microsoft windows winnt)],
+      ["linux_ubuntu",    %w(ubuntu)],
+      ["linux_chrome",    %w(chromeos)],
+      ["linux_chromium",  %w(chromiumos)],
+      ["linux_suse",      %w(suse sles)],
+      ["linux_redhat",    %w(redhat rhel)],
+      ["linux_fedora",    %w(fedora)],
+      ["linux_gentoo",    %w(gentoo)],
+      ["linux_centos",    %w(centos)],
+      ["linux_debian",    %w(debian)],
+      ["linux_coreos",    %w(coreos)],
+      ["linux_esx",       %w(vmnixx86 vmwareesxserver esxserver vmwareesxi)],
+      ["linux_solaris",   %w(solaris)],
+      ["linux_generic",   %w(linux)]
+    ]
+    # rubocop:enable Style/PercentLiteralDelimiters
+
+    # rubocop:disable Naming/VariableName
+    def self.normalize_os_name(osName)
+      findStr = osName.downcase.gsub(/[^a-z0-9]/, "")
+      @@os_map.each do |a|
+        a[1].each do |n|
+          return a[0] unless findStr.index(n).nil?
+        end
+      end
+      "unknown"
+    end
+    # rubocop:enable Naming/VariableName
+
+    # rubocop:disable Naming/VariableName
+    def self.image_name(obj)
+      osName = nil
+
+      # Select most accurate name field
+      os = obj.operating_system
+      if os
+        # check the given field names for possible matching value
+        osName = [:distribution, :product_type, :product_name].each do |field|
+          os_field = os.send(field)
+          break(os_field) if os_field && OperatingSystem.normalize_os_name(os_field) != "unknown"
+        end
+
+        # If the normalized name comes back as unknown, nil out the value so we can get it from another field
+        if osName.kind_of?(String)
+          osName = nil if OperatingSystem.normalize_os_name(osName) == "unknown"
+        else
+          osName = nil
+        end
+      end
+
+      # If the OS Name is still blank check the 'user_assigned_os'
+      if osName.nil? && obj.respond_to?(:user_assigned_os) && obj.user_assigned_os
+        osName = obj.user_assigned_os
+      end
+
+      # If the OS Name is still blank check the hardware table
+      if osName.nil? && obj.hardware && !obj.hardware.guest_os.nil?
+        osName = obj.hardware.guest_os
+        # if we get generic linux or unknown back see if the vm name is better
+        norm_os = OperatingSystem.normalize_os_name(osName)
+        if norm_os == "linux_generic" || norm_os == "unknown" # rubocop:disable Style/MultipleComparison
+          vm_name = OperatingSystem.normalize_os_name(obj.name)
+          return vm_name unless vm_name == "unknown"
+        end
+      end
+
+      # If the OS Name is still blank use the name field from the object given
+      osName = obj.name if osName.nil? && obj.respond_to?(:name)
+
+      # Normalize name to match existing icons
+      OperatingSystem.normalize_os_name(osName || "")
+    end
+    # rubocop:enable Naming/VariableName
+  end
+
+  class VmOrTemplate < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+    self.table_name = 'vms'
+
+    has_one :operating_system
+    has_one :hardware
+  end
+
+  class Host < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+
+    has_one :operating_system
+    has_one :hardware
+  end
+
+  class ComputerSystem < ActiveRecord::Base
+    has_one :operating_system
+    has_one :hardware
+  end
+
+  class Hardware < ActiveRecord::Base
+  end
+
+  def up
+    add_column :operating_systems, :platform,   :string
+    add_column :operating_systems, :image_name, :string
+
+    say_with_time("Updating platform and image_name in OperatingSystems") do
+      base_relation = OperatingSystem.all
+      say_batch_started(base_relation.size)
+
+      base_relation.find_in_batches do |group|
+        group.each(&:save)
+        say_batch_processed(group.count)
+      end
+    end
+  end
+
+  def down
+    remove_column :operating_systems, :platform,   :string
+    remove_column :operating_systems, :image_name, :string
+  end
+end
diff --git a/spec/migrations/20190108031411_add_image_name_to_operating_systems_spec.rb b/spec/migrations/20190108031411_add_image_name_to_operating_systems_spec.rb
@@ -0,0 +1,159 @@
+require_migration
+
+describe AddImageNameToOperatingSystems do
+  let(:host_stub)             { migration_stub(:Host) }
+  let(:hardware_stub)         { migration_stub(:Hardware) }
+  let(:vm_or_template_stub)   { migration_stub(:VmOrTemplate) }
+  let(:computer_system_stub)  { migration_stub(:ComputerSystem) }
+  let(:operating_system_stub) { migration_stub(:OperatingSystem) }
+
+  # rubocop:disable Layout/SpaceInsideArrayPercentLiteral
+  let(:test_os_values) do
+    [
+      %w[an_amazing_undiscovered_os unknown],
+      %w[centos-7                   linux_centos],
+      %w[debian-8                   linux_debian],
+      %w[opensuse-13                linux_suse],
+      %w[sles-12                    linux_suse],
+      %w[rhel-7                     linux_redhat],
+      %w[ubuntu-15-10               linux_ubuntu],
+      %w[windows-2012-r2            windows_generic],
+      %w[vmnix-x86                  linux_esx],
+      %w[vista                      windows_generic],
+      %w[coreos-cloud               linux_coreos]
+    ]
+  end
+  # rubocop:enable Layout/SpaceInsideArrayPercentLiteral
+
+  def record_with_os(klass, os_attributes = nil, record_attributes = {:name => ""}, hardware_attributes = nil)
+    os_record = operating_system_stub.new(os_attributes) if os_attributes
+
+    if klass == operating_system_stub
+      os_record.save!
+      os_record
+    else
+      record = klass.new
+      record_attributes.each do |attr, val|
+        record.send("#{attr}=", val) if record.respond_to?(attr)
+      end
+
+      record.operating_system = os_record
+      record.hardware         = hardware_stub.new(hardware_attributes) if hardware_attributes
+      record.save!
+      record
+    end
+  end
+
+  # Runs tests for class type to confirm they
+  def test_for_klass(klass)
+    begin
+      # This callback is necessary after the migration, but fails when the
+      # column doesn't eixst (prior to the migration).  Removing it and
+      # re-enabling it after the migration.
+      operating_system_stub.skip_callback(:save, :before, :update_platform_and_image_name)
+
+      distribution_based = []
+      product_type_based = []
+      product_name_based = []
+      fallback_records   = []
+
+      test_os_values.each do |(value, _)|
+        distribution_based << record_with_os(klass, :distribution => value)
+        product_type_based << record_with_os(klass, :product_type => value)
+        product_name_based << record_with_os(klass, :product_name => value)
+      end
+
+      # favor distribution over product_type
+      fallback_records << record_with_os(klass, :distribution => "rhel-7", :product_type => "centos-7")
+      # falls back to os.product_type if invalid os.distribution
+      fallback_records << record_with_os(klass, :distribution => "undiscovered-7", :product_type => "rhel-7")
+      # falls back to os.product_name
+      fallback_records << record_with_os(klass, :distribution => "undiscovered-7", :product_name => "rhel-7")
+      # falls back to hardware.guest_os
+      fallback_records << record_with_os(klass, {:distribution => "undiscovered-7"}, {}, {:guest_os => "rhel-7"})
+      # falls back to Host#user_assigned_os
+      fallback_records << record_with_os(klass, {:distribution => "undiscovered-7"}, {:user_assigned_os => "rhel-7"})
+    ensure
+      # If the any of the above fails, make sure we re-enable callbacks so
+      # subsequent specs don't fail trying to skip this callback when it
+      # doesn't exist.
+      operating_system_stub.set_callback(:save, :before, :update_platform_and_image_name)
+    end
+
+    migrate
+
+    test_os_values.each.with_index do |(_, image_name), index|
+      [distribution_based, product_type_based, product_name_based].each do |record_list|
+        os_record = record_list[index]
+        os_record.reload
+        os_record = os_record.operating_system if os_record.respond_to?(:operating_system)
+
+        expect(os_record.image_name).to eq(image_name)
+        expect(os_record.platform).to   eq(image_name.split("_").first)
+      end
+    end
+
+    fallback_records.each(&:reload)
+
+    platform, image_name = %w[linux linux_redhat]
+    fallback_records.each.with_index do |record, index|
+      os_record = record
+      os_record = os_record.operating_system if os_record.respond_to?(:operating_system)
+
+      # OperatingSystem records don't have a hardware relation, so this will be
+      # a "unknown" OS
+      platform, image_name = %w[unknown unknown] if index == 3 && klass == operating_system_stub
+
+      # Both ComputerSystem and VmOrTemplate don't have :user_assigned_os, so
+      # these will return "unknown" instead of what we (tried to) set.
+      platform, image_name = %w[unknown unknown] if index == 4 && klass != host_stub
+
+      expect(os_record.image_name).to eq(image_name)
+      expect(os_record.platform).to   eq(platform)
+    end
+  end
+
+  migration_context :up do
+    it "adds the columns" do
+      before_columns = operating_system_stub.columns.map(&:name)
+      expect(before_columns).to_not include("platform")
+      expect(before_columns).to_not include("image_name")
+
+      migrate
+
+      after_columns = operating_system_stub.columns.map(&:name)
+      expect(after_columns).to include("platform")
+      expect(after_columns).to include("image_name")
+    end
+
+    it "updates OperatingSystem for Host records" do
+      test_for_klass host_stub
+    end
+
+    it "updates OperatingSystem for VmOrTemplate records" do
+      test_for_klass vm_or_template_stub
+    end
+
+    it "updates OperatingSystem for ComputerSystem records" do
+      test_for_klass computer_system_stub
+    end
+
+    it "updates orphaned OperatingSystem records" do
+      test_for_klass operating_system_stub
+    end
+  end
+
+  migration_context :down do
+    it "adds the columns" do
+      before_columns = operating_system_stub.columns.map(&:name)
+      expect(before_columns).to include("platform")
+      expect(before_columns).to include("image_name")
+
+      migrate
+
+      after_columns = operating_system_stub.columns.map(&:name)
+      expect(after_columns).to_not include("platform")
+      expect(after_columns).to_not include("image_name")
+    end
+  end
+end
