diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac999e..0abd6a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## 0.6 (Unreleased) +### Added + +- Choose the model for a table explicitly with `model_classes`, or opt in to + preferring the conventionally named model with `prefer_conventional_models`, + when scripts or backfills define their own models for an existing table. + `AmbiguousModelError` names both settings + ([#135](https://github.com/rdy/fixture_builder/issues/135)). + ### Changed - Require Ruby 3.3 or newer and Rails, Active Record, and Active Support 8.0 or diff --git a/README.md b/README.md index 28c2a5e..07a41ae 100644 --- a/README.md +++ b/README.md @@ -287,6 +287,28 @@ same connection pool, and has usable primary keys. Models defined inside a migration class, such as the throwaway models data migrations use, are ignored. Independent eligible models that share a table raise `AmbiguousModelError`. +Scripts, rake tasks, and one-off backfills sometimes define their own model for +an existing table, which makes that table ambiguous once the application is +loaded. Choose the model for such a table explicitly, by class or class name: + +```ruby +FixtureBuilder.configure do |fbuilder| + fbuilder.model_classes = {"widgets" => "Widget"} +end +``` + +A mapped model must pass the same eligibility checks. Alternatively, opt in to +choosing the conventionally named model (`table_name.classify`) whenever exactly +one of the ambiguous models has that name: + +```ruby +FixtureBuilder.configure do |fbuilder| + fbuilder.prefer_conventional_models = true +end +``` + +Without either setting, ambiguity keeps raising. + FixtureBuilder writes Rails-native metadata for every model-backed file, so Rails can select the model without a separate fixture-class mapping. This also supports conventionally named models, namespaced models, and the root model for diff --git a/lib/fixture_builder/ambiguous_model_error.rb b/lib/fixture_builder/ambiguous_model_error.rb index 0dd55dc..dae764f 100644 --- a/lib/fixture_builder/ambiguous_model_error.rb +++ b/lib/fixture_builder/ambiguous_model_error.rb @@ -7,7 +7,10 @@ class AmbiguousModelError < StandardError def initialize(table_name, models) @table_name = table_name @models = models.sort_by(&:name) - super("Multiple models match table #{table_name}: #{@models.map(&:name).join(", ")}") + super( + "Multiple models match table #{table_name}: #{@models.map(&:name).join(", ")}. " \ + "Choose one with model_classes, or set prefer_conventional_models to use the conventionally named model." + ) end end end diff --git a/lib/fixture_builder/builder.rb b/lib/fixture_builder/builder.rb index 6887e8b..9b58369 100644 --- a/lib/fixture_builder/builder.rb +++ b/lib/fixture_builder/builder.rb @@ -128,7 +128,11 @@ def dump_tables private def resolve_models_by_table - resolver = ModelResolver.new(connection_pool: ActiveRecord::Base.connection_pool) + resolver = ModelResolver.new( + connection_pool: ActiveRecord::Base.connection_pool, + model_classes: model_classes, + prefer_conventional_models: prefer_conventional_models + ) tables.each_with_object({}) do |table_name, models_by_table| models_by_table[table_name] = resolver.resolve(table_name) end diff --git a/lib/fixture_builder/configuration.rb b/lib/fixture_builder/configuration.rb index 018d0bc..03d4a96 100644 --- a/lib/fixture_builder/configuration.rb +++ b/lib/fixture_builder/configuration.rb @@ -16,8 +16,10 @@ class Configuration ACCESSIBLE_ATTRIBUTES = %i[select_sql delete_sql skip_tables files_to_check record_name_fields fixture_builder_file fixture_directory after_build legacy_fixtures model_name_procs - write_empty_files].freeze - attr_accessor :after_build, :legacy_fixtures, :model_name_procs, :write_empty_files + write_empty_files model_classes prefer_conventional_models].freeze + attr_accessor :after_build, :legacy_fixtures, :model_name_procs, :write_empty_files, + :prefer_conventional_models + attr_reader :model_classes attr_writer :skip_tables, :record_name_fields, :fixture_builder_file, :fixture_directory SCHEMA_FILES = ["db/schema.rb", "db/development_structure.sql", "db/test_structure.sql", @@ -28,6 +30,14 @@ def initialize(options = {}) self.use_sha1_digests = options[:use_sha1_digests] if options.key?(:use_sha1_digests) @file_hashes = file_hashes @write_empty_files = true + @model_classes = {} + @prefer_conventional_models = false + end + + # Maps table names to the model each table's fixtures use, by class or class + # name, overriding model discovery for those tables. + def model_classes=(mapping) + @model_classes = mapping.to_h.transform_keys(&:to_s) end def include(*args) diff --git a/lib/fixture_builder/model_resolver.rb b/lib/fixture_builder/model_resolver.rb index 4a95e5b..df73a03 100644 --- a/lib/fixture_builder/model_resolver.rb +++ b/lib/fixture_builder/model_resolver.rb @@ -2,11 +2,15 @@ module FixtureBuilder class ModelResolver - def initialize(connection_pool:) + def initialize(connection_pool:, model_classes: {}, prefer_conventional_models: false) @connection_pool = connection_pool + @model_classes = model_classes.to_h.transform_keys(&:to_s) + @prefer_conventional_models = prefer_conventional_models end def resolve(table_name) + return mapped_model(table_name) if @model_classes.key?(table_name) + # Prime conventional autoloading before considering already-loaded models. table_name.classify.safe_constantize candidates = ActiveRecord::Base.descendants.select do |model| @@ -19,11 +23,23 @@ def resolve(table_name) return if root_models.empty? return root_models.first if root_models.one? + conventional_model = root_models.find { |model| model.name == table_name.classify } + return conventional_model if @prefer_conventional_models && conventional_model + raise AmbiguousModelError.new(table_name, root_models) end private + def mapped_model(table_name) + mapping = @model_classes.fetch(table_name) + model = mapping.is_a?(Module) ? mapping : mapping.to_s.constantize + return model if model < ActiveRecord::Base && eligible_model?(model, table_name) + + raise ArgumentError, "model_classes maps #{table_name} to #{model.name}, which is not an eligible model " \ + "for that table: it must be concrete, named, use the table on the same connection, and have a primary key" + end + def eligible_model?(model, table_name) return false if model.abstract_class? diff --git a/test/model_resolver/ambiguous_model_error_test.rb b/test/model_resolver/ambiguous_model_error_test.rb index e8441bf..fed94a2 100644 --- a/test/model_resolver/ambiguous_model_error_test.rb +++ b/test/model_resolver/ambiguous_model_error_test.rb @@ -14,7 +14,8 @@ def test_ambiguous_model_error_exposes_its_table_name_and_models assert_equal "creatures", error.table_name assert_equal [generated_creature, magical_creature], error.models assert_equal( - "Multiple models match table creatures: GeneratedCreature, MagicalCreature", + "Multiple models match table creatures: GeneratedCreature, MagicalCreature. " \ + "Choose one with model_classes, or set prefer_conventional_models to use the conventionally named model.", error.message ) end diff --git a/test/model_resolver/conventional_preference_test.rb b/test/model_resolver/conventional_preference_test.rb new file mode 100644 index 0000000..2423ca3 --- /dev/null +++ b/test/model_resolver/conventional_preference_test.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require_relative "../test_helper" + +module ModelResolverTests + class ConventionalPreferenceTest < Test::Unit::TestCase + prepend IsolatedFixtureFilesystem + + with_table :gizmos do |table| + table.string :name + end + + with_model :Gizmo do + table(false) + + model do + self.table_name = "gizmos" + end + end + + with_model :Widget do + table { |table| table.string :name } + end + + def setup + gizmo_backfill = Class.new + ModelResolverTests.const_set(:GizmoBackfill, gizmo_backfill) + gizmo_backfill.const_set(:GizmoRow, Class.new(ActiveRecord::Base) { self.table_name = "gizmos" }) + + widget_backfill = Class.new + ModelResolverTests.const_set(:WidgetBackfill, widget_backfill) + widget_table = Widget.table_name + widget_backfill.const_set(:WidgetRow, Class.new(ActiveRecord::Base) { self.table_name = widget_table }) + end + + def teardown + %i[GizmoBackfill WidgetBackfill].each do |name| + ModelResolverTests.send(:remove_const, name) if ModelResolverTests.const_defined?(name, false) + end + end + + def test_conventional_models_are_not_preferred_by_default + assert_raise(FixtureBuilder::AmbiguousModelError) { resolve_model("gizmos") } + end + + def test_preference_chooses_the_conventionally_named_model + assert_equal Gizmo, resolve_model("gizmos", prefer_conventional_models: true) + end + + def test_preference_still_raises_without_a_conventionally_named_model + assert_raise(FixtureBuilder::AmbiguousModelError) do + resolve_model(Widget.table_name, prefer_conventional_models: true) + end + end + + def test_configured_preference_applies_to_the_build + force_fixture_generation + + FixtureBuilder.configure do |fbuilder| + fbuilder.files_to_check = [] + fbuilder.skip_tables = ActiveRecord::Base.connection.tables - ["gizmos"] + fbuilder.prefer_conventional_models = true + fbuilder.factory { Gizmo.create!(name: "Flux capacitor") } + end + + fixture = YAML.safe_load_file(fixture_path("gizmos.yml")) + assert_equal({"model_class" => "Gizmo"}, fixture["_fixture"]) + end + + private + + def resolve_model(table_name, **options) + FixtureBuilder::ModelResolver.new(connection_pool: ActiveRecord::Base.connection_pool, **options).resolve(table_name) + end + end +end diff --git a/test/model_resolver/model_classes_test.rb b/test/model_resolver/model_classes_test.rb new file mode 100644 index 0000000..7af69b2 --- /dev/null +++ b/test/model_resolver/model_classes_test.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require_relative "../test_helper" + +module ModelResolverTests + class ModelClassesTest < Test::Unit::TestCase + prepend IsolatedFixtureFilesystem + + with_model :Widget do + table { |table| table.string :name } + end + + with_model :Gadget do + table { |table| table.string :name } + end + + def setup + # Scripts and one-off backfills often define a row model for an existing + # table, which makes the table ambiguous once the application is loaded. + backfill = Class.new + ModelResolverTests.const_set(:WidgetBackfill, backfill) + table_name = Widget.table_name + backfill.const_set(:WidgetRow, Class.new(ActiveRecord::Base) { self.table_name = table_name }) + end + + def teardown + ModelResolverTests.send(:remove_const, :WidgetBackfill) if ModelResolverTests.const_defined?(:WidgetBackfill, false) + end + + def test_unrelated_models_for_one_table_raise_by_default + error = assert_raise(FixtureBuilder::AmbiguousModelError) { resolve_model(Widget.table_name) } + + assert_match "model_classes", error.message + end + + def test_model_classes_chooses_the_mapped_model + assert_equal Widget, resolve_model(Widget.table_name, model_classes: {Widget.table_name => "Widget"}) + end + + def test_configured_model_classes_apply_to_the_build + table_name = Widget.table_name + force_fixture_generation + + FixtureBuilder.configure do |fbuilder| + fbuilder.files_to_check = [] + fbuilder.skip_tables = ActiveRecord::Base.connection.tables - [table_name] + fbuilder.model_classes = {table_name.to_sym => Widget} + fbuilder.factory { Widget.create!(name: "Sprocket") } + end + + fixture = YAML.safe_load_file(fixture_path("#{table_name}.yml")) + assert_equal({"model_class" => "Widget"}, fixture["_fixture"]) + end + + def test_a_mapped_model_for_another_table_raises + error = assert_raise(ArgumentError) do + resolve_model(Widget.table_name, model_classes: {Widget.table_name => "Gadget"}) + end + + assert_match "Gadget", error.message + end + + private + + def resolve_model(table_name, **options) + FixtureBuilder::ModelResolver.new(connection_pool: ActiveRecord::Base.connection_pool, **options).resolve(table_name) + end + end +end diff --git a/test/support/model_resolver_ambiguity_behavior.rb b/test/support/model_resolver_ambiguity_behavior.rb index a56faf7..6443836 100644 --- a/test/support/model_resolver_ambiguity_behavior.rb +++ b/test/support/model_resolver_ambiguity_behavior.rb @@ -21,7 +21,8 @@ def test_unrelated_models_raise_before_replacing_the_fixture assert_equal table_name, error.table_name assert_equal %w[Alpha Beta], error.models.map(&:name) assert_equal( - "Multiple models match table #{table_name}: Alpha, Beta", + "Multiple models match table #{table_name}: Alpha, Beta. " \ + "Choose one with model_classes, or set prefer_conventional_models to use the conventionally named model.", error.message ) assert_equal original_fixture, File.binread(fixture_path)