diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ea5dac..f25dd1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,9 @@ ### Fixed +- Omit database-generated columns from generated fixtures so Rails can load + snapshots from tables that have them + ([#100](https://github.com/rdy/fixture_builder/issues/100)). - Generate model-backed fixture rows in primary-key order so fixture output remains stable ([#50](https://github.com/rdy/fixture_builder/pull/50); thanks [jackkinsella](https://github.com/jackkinsella)). diff --git a/README.md b/README.md index fab0473..b56259e 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,8 @@ By default these are set as: * select_sql: SELECT * FROM %s * delete_sql: DELETE FROM %
s +FixtureBuilder omits database-generated columns from snapshots because Rails fixtures cannot write them; writable columns remain included. + FixtureBuilder supports Ruby's two [reference by name](https://docs.ruby-lang.org/en/3.3/format_specifications_rdoc.html#label-Reference+by+Name) forms for the table placeholder. In `%
s`, `table` is the named key and @@ -174,6 +176,10 @@ format("SELECT * FROM %
s", table: '"users"') # => "SELECT * FROM \"users\ format("DELETE FROM %{table}", table: '"users"') # => "DELETE FROM \"users\"" ``` +FixtureBuilder omits database-generated columns from snapshots because Rails +fixtures cannot write them. Every other selected field is kept, including +aliases and computed values introduced by a custom `select_sql`. + Positional `%s` placeholders were deprecated in FixtureBuilder 0.5.0 and are rejected in 0.6. Assigning `select_sql` or `delete_sql` is deprecated but remains supported throughout 0.6. The setters are planned for removal in 0.7 unless users diff --git a/lib/fixture_builder/builder.rb b/lib/fixture_builder/builder.rb index 8b65e3e..55d8063 100644 --- a/lib/fixture_builder/builder.rb +++ b/lib/fixture_builder/builder.rb @@ -103,6 +103,7 @@ def dump_tables nil end rows = if table_klass && table_klass < ActiveRecord::Base + generated_names = generated_column_names(table_klass.table_name) table_klass.unscoped do table_klass.order(:id).all.collect do |obj| attrs = obj.attributes_before_type_cast.slice(*table_klass.column_names) @@ -112,12 +113,14 @@ def dump_tables attrs[attr_name] = JSON.parse(value) end - attrs + attrs.except(*generated_names) end end else + generated_names = generated_column_names(table_name) ActiveRecord::Base.connection.select_all(format(select_sql, table: ActiveRecord::Base.connection.quote_table_name(table_name))) + .map { |row| row.except(*generated_names) } end next files if rows.empty? @@ -135,6 +138,17 @@ def dump_tables say "Built #{fixtures.to_sentence}" end + # Database-generated (virtual/stored generated) columns cannot be written + # back, so Rails rejects fixtures containing them. Only these names are + # removed, so custom `select_sql` aliases still reach Rails and fail loudly + # rather than disappearing silently. + private def generated_column_names(table_name) + connection = ActiveRecord::Base.connection + return [] unless connection.supports_virtual_columns? + + connection.columns(table_name).select(&:virtual?).map(&:name) + end + def write_fixture_file(fixture_data, table_name) File.write(fixture_file(table_name), fixture_data.to_yaml) end diff --git a/test/fixture_builder_test.rb b/test/fixture_builder_test.rb index 53f9b17..3887b2a 100644 --- a/test/fixture_builder_test.rb +++ b/test/fixture_builder_test.rb @@ -9,6 +9,8 @@ def self.table_name end class FixtureBuilderTest < Test::Unit::TestCase + include GeneratedFixtureSchema + def teardown FixtureBuilder.instance_variable_set(:@configuration, nil) end @@ -67,6 +69,96 @@ def test_do_not_include_virtual_attributes assert !generated_fixture["uni"].key?("virtual") end + def test_generated_columns_are_excluded_for_model_backed_tables + create_and_blow_away_old_db + force_fixture_generation + + table_name = GeneratedCreature.table_name + FixtureBuilder.configure do |fbuilder| + fbuilder.files_to_check = [] + fbuilder.skip_tables = ActiveRecord::Base.connection.tables - [table_name] + fbuilder.factory { GeneratedCreature.create!(name: "Myrddin") } + end + + generated_fixture = YAML.safe_load_file(test_path("fixtures/#{table_name}.yml")) + assert_equal "Myrddin", generated_fixture.dig("myrddin", "name") + assert_not_include generated_fixture.fetch("myrddin"), "name_length" + + GeneratedCreature.delete_all + create_fixtures(table_name) + assert_equal 7, GeneratedCreature.find_by!(name: "Myrddin").name_length + end + + def test_generated_columns_are_excluded_for_raw_query_tables + create_and_blow_away_old_db + force_fixture_generation + + table_name = GENERATED_COLUMN_RECORDS_TABLE + FixtureBuilder.configure do |fbuilder| + fbuilder.files_to_check = [] + fbuilder.skip_tables = ActiveRecord::Base.connection.tables - [table_name] + fbuilder.factory do + ActiveRecord::Base.connection.execute( + "INSERT INTO #{table_name} (name) VALUES ('Merlin')" + ) + end + end + + generated_fixture = YAML.safe_load_file(test_path("fixtures/#{table_name}.yml")) + assert_equal "Merlin", generated_fixture.dig("merlin", "name") + assert_not_include generated_fixture.fetch("merlin"), "name_length" + + ActiveRecord::Base.connection.delete("DELETE FROM #{table_name}") + create_fixtures(table_name) + assert_equal 6, + ActiveRecord::Base.connection.select_value("SELECT name_length FROM #{table_name}") + end + + def test_raw_query_select_aliases_survive_generated_column_filtering + create_and_blow_away_old_db + force_fixture_generation + + table_name = GENERATED_COLUMN_RECORDS_TABLE + capture_output do + FixtureBuilder.configure do |fbuilder| + fbuilder.files_to_check = [] + fbuilder.skip_tables = ActiveRecord::Base.connection.tables - [table_name] + fbuilder.select_sql = "SELECT *, upper(name) AS shouted_name FROM %
s" + fbuilder.factory do + ActiveRecord::Base.connection.execute( + "INSERT INTO #{table_name} (name) VALUES ('Merlin')" + ) + end + end + end + + generated_fixture = YAML.safe_load_file(test_path("fixtures/#{table_name}.yml")) + record = generated_fixture.fetch("merlin") + assert_equal "MERLIN", record["shouted_name"] + assert_not_include record, "name_length" + end + + def test_writable_columns_come_from_the_model_table_name + create_and_blow_away_old_db + force_fixture_generation + + table_name = RELOCATED_CREATURES_TABLE + FixtureBuilder.configure do |fbuilder| + fbuilder.files_to_check = [] + fbuilder.skip_tables = ActiveRecord::Base.connection.tables - [table_name] + fbuilder.factory { RelocatedCreature.create!(name: "Nimue") } + end + + generated_fixture = YAML.safe_load_file(test_path("fixtures/#{table_name}.yml")) + # `name` is writable on the model's own table, so it must survive even + # though the iterated table of the same inferred name generates it. + assert_include generated_fixture, "nimue" + record = generated_fixture.fetch("nimue") + assert_include record, "name" + assert_equal "Nimue", record["name"] + assert_not_include record, "unrelated" + end + def test_custom_json_attribute_type_round_trips_through_fixtures create_and_blow_away_old_db force_fixture_generation diff --git a/test/legacy_fixture_mode_fixture_generation_test.rb b/test/legacy_fixture_mode_fixture_generation_test.rb index 41d756c..87855bc 100644 --- a/test/legacy_fixture_mode_fixture_generation_test.rb +++ b/test/legacy_fixture_mode_fixture_generation_test.rb @@ -3,6 +3,8 @@ require File.expand_path(File.join(File.dirname(__FILE__), "test_helper")) class LegacyFixtureModeFixtureGenerationTest < Test::Unit::TestCase + include GeneratedFixtureSchema + def setup create_and_blow_away_old_db force_fixture_generation diff --git a/test/legacy_fixture_mode_test.rb b/test/legacy_fixture_mode_test.rb index 2ceca93..8073eae 100644 --- a/test/legacy_fixture_mode_test.rb +++ b/test/legacy_fixture_mode_test.rb @@ -3,6 +3,8 @@ require File.expand_path(File.join(File.dirname(__FILE__), "test_helper")) class LegacyFixtureModeTest < Test::Unit::TestCase + include GeneratedFixtureSchema + def setup create_and_blow_away_old_db force_fixture_generation diff --git a/test/namer_test.rb b/test/namer_test.rb index f946bac..dcb2b22 100644 --- a/test/namer_test.rb +++ b/test/namer_test.rb @@ -23,6 +23,8 @@ def self.model_class = NamerTestModel end class NamerTest < Test::Unit::TestCase + include GeneratedFixtureSchema + def setup configuration = FixtureBuilder::Configuration.new @namer = FixtureBuilder::Namer.new(configuration) diff --git a/test/support/test_database.rb b/test/support/test_database.rb new file mode 100644 index 0000000..c945a79 --- /dev/null +++ b/test/support/test_database.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# Shared test database used by the FixtureBuilder tests. +# +# Test classes include this module and call +create_and_blow_away_old_db+ from +# their own +setup+ or from individual tests, exactly as they did when this +# lived as a top-level helper method. +module TestDatabase + CONFIGURATION = {"adapter" => "sqlite3", "database" => ":memory:"}.freeze + + def create_and_blow_away_old_db + ActiveRecord::Base.configurations = {"test" => CONFIGURATION} + ActiveRecord::Base.establish_connection(:test) + ActiveRecord::Base.connection.create_table(:magical_creatures, force: true) do |t| + t.column :name, :string + t.column :species, :string + t.column :powers, :string + t.column :wizard_data, :json + t.column :deleted, :boolean, default: false, null: false + end + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index b02e24e..a6e1b3e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -34,15 +34,16 @@ def create_fixtures(*table_names, &block) # rewritten YAML is reparsed; Rails 8.0 and 8.1 use the original load path. if fixture_set.respond_to?(:without_parsing_cache) fixture_set.without_parsing_cache do - fixture_set.create_fixtures(ActiveSupport::TestCase.fixture_path, table_names, {}, &block) + fixture_set.create_fixtures(test_path("fixtures"), table_names, {}, &block) end else - fixture_set.create_fixtures(ActiveSupport::TestCase.fixture_path, table_names, {}, &block) + fixture_set.create_fixtures(test_path("fixtures"), table_names, {}, &block) end end require "sqlite3" require "fixture_builder" +require_relative "support/test_database" class WizardData attr_reader :level, :title, :allies @@ -95,6 +96,16 @@ def wizard_data(attributes) end # standard:disable Rails/ApplicationRecord +class GeneratedCreature < ActiveRecord::Base +end + +# Inferable from the `relocated_creatures` table name, but backed by a +# differently named table, so writable column names must come from +# `table_name` rather than the table FixtureBuilder is iterating. +class RelocatedCreature < ActiveRecord::Base + self.table_name = "creature_archive" +end + class MagicalCreature < ActiveRecord::Base validates_presence_of :name, :species serialize :powers, type: Array @@ -106,17 +117,87 @@ class MagicalCreature < ActiveRecord::Base end # standard:enable Rails/ApplicationRecord -def create_and_blow_away_old_db - ActiveRecord::Base.configurations = {"test" => {"adapter" => "sqlite3", "database" => "test.db"}} +# Shared test schema for FixtureBuilder's database-generated column handling. +# +# Builds on TestDatabase: `super` creates the connection and the +# `magical_creatures` table, and this module adds the tables that exist only to +# exercise database-generated columns. +# +# Ownership of the schema and of the fixture files it causes FixtureBuilder to +# write live together: including this module registers a teardown that removes +# those fixture files, so every test class that builds the shared schema cleans +# up after itself rather than depending on another class running later. +module GeneratedFixtureSchema + include TestDatabase + + GENERATED_CREATURES_TABLE = "generated_creatures" + GENERATED_COLUMN_RECORDS_TABLE = "generated_column_records" + RELOCATED_CREATURES_TABLE = "relocated_creatures" + CREATURE_ARCHIVE_TABLE = "creature_archive" + + # The only tables created solely to exercise generated columns, and therefore + # the only fixture files a run is allowed to delete. `magical_creatures` is + # user-authored fixture data and is deliberately absent. + GENERATED_TEST_TABLES = [ + GENERATED_CREATURES_TABLE, + GENERATED_COLUMN_RECORDS_TABLE, + RELOCATED_CREATURES_TABLE, + CREATURE_ARCHIVE_TABLE + ].freeze + + def self.included(base) + base.teardown :clean_up_generated_fixture_files + end - ActiveRecord::Base.establish_connection(:test) + def create_and_blow_away_old_db + super + + connection = ActiveRecord::Base.connection + + # Inferable as the `GeneratedCreature` model, so the model-backed extraction + # path sees a database-generated column. + create_generated_column_table(GENERATED_CREATURES_TABLE) + + # No inferable model, so the raw-query extraction path sees a + # database-generated column. + create_generated_column_table(GENERATED_COLUMN_RECORDS_TABLE) + + # The table FixtureBuilder iterates (`relocated_creatures`) alongside the + # differently named table `RelocatedCreature` actually reads. + # + # The two tables expose deliberately incompatible schemas: the iterated + # table's only writable column is `unrelated` and its `name` is + # database-generated, while the model's table has a writable `name`. Reading + # generated columns from the iterated table instead of the model's table + # therefore strips `name` from the fixture. + connection.create_table(RELOCATED_CREATURES_TABLE, force: true) do |t| + t.string :unrelated + t.virtual :name, type: :string, as: "upper(unrelated)", stored: true + end + connection.create_table(CREATURE_ARCHIVE_TABLE, force: true) do |t| + t.string :name, null: false + end - ActiveRecord::Base.connection.create_table(:magical_creatures, force: true) do |t| - t.column :name, :string - t.column :species, :string - t.column :powers, :string - t.column :wizard_data, :json - t.column :deleted, :boolean, default: false, null: false + GeneratedCreature.reset_column_information + RelocatedCreature.reset_column_information + end + + # Creates a table with a writable `name` column and a stored + # database-generated `name_length` column. + def create_generated_column_table(table_name) + ActiveRecord::Base.connection.create_table(table_name, force: true) do |t| + t.string :name, null: false + t.virtual :name_length, type: :integer, as: "length(name)", stored: true + end + end + + # FixtureBuilder writes a fixture file for every table it iterates, so runs + # leave behind output for the generated-column tables above. Delete exactly + # those files, never the fixture directory or fixtures the repository owns. + def clean_up_generated_fixture_files + GENERATED_TEST_TABLES.each do |table_name| + FileUtils.rm_f(test_path("fixtures/#{table_name}.yml")) + end end end