From 7c89a2f14843ebf2e4394f643e2761f2ccf84985 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Thu, 25 Jun 2026 10:12:00 +0200 Subject: [PATCH 1/3] Fixes #39465 - Migrate ignore_types from YAML text to JSONB PR #10990 review feedback suggested replacing the fragile YAML-based potentially_ignoring scope. This migration provides: - **Exact matching**: JSONB containment operator (@>) eliminates over-matching (e.g., 'User' no longer matches 'UserGroup') - **No post-filter**: Previously required Ruby .ignore? check is gone - **Indexed queries**: GIN index enables O(1) lookup vs sequential scan - **Format independence**: No coupling to YAML serialization Renamed scope from potentially_ignoring to ignoring since JSONB containment provides exact matching - no "potentially" about it. Added comprehensive test coverage for the scope including exact matching, no over-matching, empty arrays, and multiple types. Co-Authored-By: Claude Opus 4.6 --- app/models/taxonomy.rb | 7 ++- ...igrate_taxonomies_ignore_types_to_jsonb.rb | 41 +++++++++++++++ test/models/taxonomy_test.rb | 50 ++++++++++++++++++- 3 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20260624141920_migrate_taxonomies_ignore_types_to_jsonb.rb diff --git a/app/models/taxonomy.rb b/app/models/taxonomy.rb index 178914a994..7a596253db 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.classify].to_json) } before_create :assign_default_templates after_create :assign_taxonomy_to_user 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..dcd225f807 --- /dev/null +++ b/db/migrate/20260624141920_migrate_taxonomies_ignore_types_to_jsonb.rb @@ -0,0 +1,41 @@ +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, Array, 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| + taxonomy.update_column(:ignore_types, taxonomy.ignore_types_yaml || []) + 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/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 From 2c90353ae9c7884745286697e6bd0aefd8c5894e Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Thu, 25 Jun 2026 10:12:06 +0200 Subject: [PATCH 2/3] Refs #39465 - Update ignore_types callers for JSONB - Pass User class directly to ignoring scope instead of string 'user' - Replace LIKE queries in cleanup_helper with ignoring scope Co-Authored-By: Claude Opus 4.6 --- app/models/user.rb | 2 +- lib/cleanup_helper.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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/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| From 94db28f685eced1d166cba3b924fe1bc743ad171 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Mon, 29 Jun 2026 15:08:20 +0200 Subject: [PATCH 3/3] Refs #39465 - Address review feedback on ignore_types JSONB migration - Fix serialize syntax in migration fake model to be Rails 7.1+ compatible - Guard against malformed YAML data during migration with a warning log - Remove silent .classify normalization from ignoring scope Co-Authored-By: Claude Sonnet 4.6 --- app/models/taxonomy.rb | 2 +- ...624141920_migrate_taxonomies_ignore_types_to_jsonb.rb | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/models/taxonomy.rb b/app/models/taxonomy.rb index 7a596253db..5ad74f3762 100644 --- a/app/models/taxonomy.rb +++ b/app/models/taxonomy.rb @@ -7,7 +7,7 @@ class Taxonomy < ApplicationRecord # 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.classify].to_json) } + 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/db/migrate/20260624141920_migrate_taxonomies_ignore_types_to_jsonb.rb b/db/migrate/20260624141920_migrate_taxonomies_ignore_types_to_jsonb.rb index dcd225f807..3090f998c6 100644 --- a/db/migrate/20260624141920_migrate_taxonomies_ignore_types_to_jsonb.rb +++ b/db/migrate/20260624141920_migrate_taxonomies_ignore_types_to_jsonb.rb @@ -3,7 +3,7 @@ class MigrateTaxonomiesIgnoreTypesToJsonb < ActiveRecord::Migration[7.0] class MigrationTaxonomy < ApplicationRecord self.table_name = 'taxonomies' self.inheritance_column = :_type_disabled - serialize :ignore_types_yaml, Array, coder: YAML + serialize :ignore_types_yaml, coder: YAML end def up @@ -27,7 +27,12 @@ def down def migrate_yaml_to_jsonb MigrationTaxonomy.reset_column_information MigrationTaxonomy.find_each do |taxonomy| - taxonomy.update_column(:ignore_types, taxonomy.ignore_types_yaml || []) + 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