diff --git a/app/models/taxonomy.rb b/app/models/taxonomy.rb index 178914a994..5ad74f3762 100644 --- a/app/models/taxonomy.rb +++ b/app/models/taxonomy.rb @@ -5,10 +5,9 @@ class Taxonomy < ApplicationRecord include NestedAncestryCommon include TopbarCacheExpiry - serialize :ignore_types, Array - # Coarse SQL pre-filter on serialized YAML; may over-match (e.g. 'User' matches 'UserProfile'). - # Callers must verify with ignore? for exactness. - scope :potentially_ignoring, ->(type) { where("ignore_types LIKE ?", "%#{sanitize_sql_like(type.to_s.classify)}%") } + # ignore_types is a JSONB column storing an array of class names (e.g. ["User", "Domain"]) + # JSONB provides exact matching via containment operator and supports GIN indexing for O(1) lookup + scope :ignoring, ->(type) { where("ignore_types @> ?", [type.to_s].to_json) } before_create :assign_default_templates after_create :assign_taxonomy_to_user diff --git a/app/models/user.rb b/app/models/user.rb index e0e6b49ebd..d7c4e52ba3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -516,7 +516,7 @@ def taxonomy_and_child_ids(taxonomies) delay = Rails.env.test? ? 0 : 2.minutes Rails.cache.fetch("user/#{id}/taxonomy_and_child_ids/#{taxonomies}", expires_in: delay) do klass = taxonomies.to_s.classify.constantize - top_level = send(taxonomies) + klass.unscoped.potentially_ignoring('user').select { |tax| tax.ignore?('user') } + top_level = send(taxonomies) + klass.unscoped.ignoring(User) next [] if top_level.empty? Taxonomy.batch_subtree_ids(top_level) diff --git a/db/migrate/20260624141920_migrate_taxonomies_ignore_types_to_jsonb.rb b/db/migrate/20260624141920_migrate_taxonomies_ignore_types_to_jsonb.rb new file mode 100644 index 0000000000..3090f998c6 --- /dev/null +++ b/db/migrate/20260624141920_migrate_taxonomies_ignore_types_to_jsonb.rb @@ -0,0 +1,46 @@ +class MigrateTaxonomiesIgnoreTypesToJsonb < ActiveRecord::Migration[7.0] + # Fake model to isolate migration from future model changes + class MigrationTaxonomy < ApplicationRecord + self.table_name = 'taxonomies' + self.inheritance_column = :_type_disabled + serialize :ignore_types_yaml, coder: YAML + end + + def up + rename_column :taxonomies, :ignore_types, :ignore_types_yaml + add_column :taxonomies, :ignore_types, :jsonb, default: [] + migrate_yaml_to_jsonb + remove_column :taxonomies, :ignore_types_yaml + add_index :taxonomies, :ignore_types, using: :gin + end + + def down + remove_index :taxonomies, :ignore_types + add_column :taxonomies, :ignore_types_yaml, :text + migrate_jsonb_to_yaml + remove_column :taxonomies, :ignore_types + rename_column :taxonomies, :ignore_types_yaml, :ignore_types + end + + private + + def migrate_yaml_to_jsonb + MigrationTaxonomy.reset_column_information + MigrationTaxonomy.find_each do |taxonomy| + value = taxonomy.ignore_types_yaml + unless value.is_a?(Array) || value.nil? + Rails.logger.warn("MigrateTaxonomiesIgnoreTypesToJsonb: taxonomy #{taxonomy.id} had unexpected ignore_types value #{value.class}, resetting to []") + value = [] + end + taxonomy.update_column(:ignore_types, value || []) + end + end + + def migrate_jsonb_to_yaml + MigrationTaxonomy.reset_column_information + MigrationTaxonomy.find_each do |taxonomy| + value = taxonomy.ignore_types || [] + taxonomy.update_column(:ignore_types_yaml, value.empty? ? nil : value.to_yaml) + end + end +end diff --git a/lib/cleanup_helper.rb b/lib/cleanup_helper.rb index d75cf89053..8c430cee1e 100644 --- a/lib/cleanup_helper.rb +++ b/lib/cleanup_helper.rb @@ -71,8 +71,8 @@ def clean_puppet Feature.where(name: 'Puppet').destroy_all - organizations = Organization.where("ignore_types LIKE '%Environment%'") - locations = Location.where("ignore_types LIKE '%Environment%'") + organizations = Organization.ignoring('Environment') + locations = Location.ignoring('Environment') User.as_anonymous_admin do (organizations + locations).each do |tax| diff --git a/test/models/taxonomy_test.rb b/test/models/taxonomy_test.rb index 806366ce40..147ef26a10 100644 --- a/test/models/taxonomy_test.rb +++ b/test/models/taxonomy_test.rb @@ -103,11 +103,11 @@ class TaxonomyTest < ActiveSupport::TestCase assert_equal result.sort, result end - test 'potentially_ignoring returns taxonomies that ignore the given type' do + test 'ignoring returns taxonomies that ignore the given type' do ignoring = FactoryBot.create(:organization, ignore_types: ['User']) not_ignoring = FactoryBot.create(:organization, ignore_types: []) - result = Organization.unscoped.potentially_ignoring('user') + result = Organization.unscoped.ignoring(User) assert_includes result, ignoring refute_includes result, not_ignoring end @@ -148,4 +148,50 @@ class TaxonomyTest < ActiveSupport::TestCase location.save assert_match /expecting organizations/, location.errors.messages[:organizations].first end + + test 'ignoring finds taxonomies with exact type match' do + org_with_user = FactoryBot.create(:organization, ignore_types: ['User', 'Domain']) + org_with_host = FactoryBot.create(:organization, ignore_types: ['Host']) + org_empty = FactoryBot.create(:organization, ignore_types: []) + + result = Organization.ignoring('User') + assert_includes result, org_with_user + refute_includes result, org_with_host + refute_includes result, org_empty + end + + test 'ignoring does not over-match similar class names' do + # JSONB containment prevents over-matching (e.g. 'User' should NOT match 'UserGroup') + org_user = FactoryBot.create(:organization, ignore_types: ['User']) + org_usergroup = FactoryBot.create(:organization, ignore_types: ['UserGroup']) + + result_user = Organization.ignoring('User') + assert_includes result_user, org_user + refute_includes result_user, org_usergroup + + result_usergroup = Organization.ignoring('UserGroup') + assert_includes result_usergroup, org_usergroup + refute_includes result_usergroup, org_user + end + + test 'ignoring handles empty ignore_types' do + org_with_types = FactoryBot.create(:organization, ignore_types: ['User']) + org_empty = FactoryBot.create(:organization, ignore_types: []) + org_nil = FactoryBot.create(:organization) + org_nil.update_column(:ignore_types, nil) unless org_nil.ignore_types.nil? + + result = Organization.ignoring('User') + assert_includes result, org_with_types + refute_includes result, org_empty + refute_includes result, org_nil + end + + test 'ignoring works with multiple types' do + org = FactoryBot.create(:organization, ignore_types: ['User', 'Domain', 'Subnet']) + + assert_includes Organization.ignoring('User'), org + assert_includes Organization.ignoring('Domain'), org + assert_includes Organization.ignoring('Subnet'), org + refute_includes Organization.ignoring('Host'), org + end end