From 58369e1d8fe16ae7f37dee3da05c120827afdf3e Mon Sep 17 00:00:00 2001 From: lorint Date: Mon, 30 Jul 2018 15:50:32 +0100 Subject: [PATCH] Fix for issue #594, reifying sub-classed models that use STI (#1108) See the changes to the changelog and readme for details. --- CHANGELOG.md | 14 +- README.md | 97 ++++++++++-- .../paper_trail/{ => install}/USAGE | 3 +- .../{ => install}/install_generator.rb | 42 +---- .../add_object_changes_to_versions.rb.erb | 0 .../templates/create_versions.rb.erb | 2 +- .../paper_trail/migration_generator.rb | 37 +++++ lib/generators/paper_trail/update_sti/USAGE | 4 + .../templates/update_versions_for_sti.rb.erb | 85 +++++++++++ .../update_sti/update_sti_generator.rb | 17 +++ lib/paper_trail/config.rb | 10 +- lib/paper_trail/events/destroy.rb | 2 +- lib/paper_trail/model_config.rb | 32 +++- lib/paper_trail/record_trail.rb | 69 +++++++-- lib/paper_trail/reifier.rb | 36 ++--- .../app/models/family/celebrity_family.rb | 6 + .../config/initializers/paper_trail.rb | 1 + .../20110208155312_set_up_test_tables.rb | 2 + .../paper_trail/install_generator_spec.rb | 11 +- spec/models/animal_spec.rb | 6 + spec/models/cat_spec.rb | 29 ++++ spec/models/family/celebrity_family_spec.rb | 122 +++++++++++++++ spec/models/person_spec.rb | 27 +++- spec/models/pet_spec.rb | 143 +++++++++++++++++- spec/paper_trail/events/destroy_spec.rb | 21 +++ spec/spec_helper.rb | 4 +- spec/support/alt_db_init.rb | 5 +- spec/support/paper_trail_spec_migrator.rb | 57 ++++++- 28 files changed, 773 insertions(+), 111 deletions(-) rename lib/generators/paper_trail/{ => install}/USAGE (52%) rename lib/generators/paper_trail/{ => install}/install_generator.rb (68%) rename lib/generators/paper_trail/{ => install}/templates/add_object_changes_to_versions.rb.erb (100%) rename lib/generators/paper_trail/{ => install}/templates/create_versions.rb.erb (94%) create mode 100644 lib/generators/paper_trail/migration_generator.rb create mode 100644 lib/generators/paper_trail/update_sti/USAGE create mode 100644 lib/generators/paper_trail/update_sti/templates/update_versions_for_sti.rb.erb create mode 100644 lib/generators/paper_trail/update_sti/update_sti_generator.rb create mode 100644 spec/dummy_app/app/models/family/celebrity_family.rb create mode 100644 spec/models/cat_spec.rb create mode 100644 spec/models/family/celebrity_family_spec.rb create mode 100644 spec/paper_trail/events/destroy_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a3893dd8..334dd8164 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,16 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Breaking Changes -- None +- [#1108](https://github.com/paper-trail-gem/paper_trail/pull/1108) - + In `versions.item_type`, we now store the subclass name instead of + the base_class. + - You must migrate existing `versions` records if you use + [STI][1]. A migration generator has been provided. Generator `update_sti` + creates a migration that updates existing `version` entries such that + `item_type` then refers to the specific class name instead of base_class. + See [5.c. Generators][2] for instructions. + - This change fixes a long-standing issue with reification of STI subclasses, + [#594](https://github.com/paper-trail-gem/paper_trail/issues/594) ### Added @@ -1022,3 +1031,6 @@ in the `PaperTrail::Version` class through a `Rails::Engine` when the gem is use - [#160](https://github.com/paper-trail-gem/paper_trail/pull/160) - Fixed failing tests and resolved out of date dependency issues. - [#157](https://github.com/paper-trail-gem/paper_trail/pull/157) - Refactored `class_attribute` names on the `ClassMethods` module for names that are not obviously pertaining to PaperTrail to prevent method name collision. + +[1]: https://api.rubyonrails.org/v5.2.0/classes/ActiveRecord/Base.html#class-ActiveRecord::Base-label-Single+table+inheritance +[2]: https://github.com/paper-trail-gem/paper_trail/blob/master/README.md#5c-generators diff --git a/README.md b/README.md index 512245014..c76e97bf7 100644 --- a/README.md +++ b/README.md @@ -941,8 +941,15 @@ class Banana < Fruit end ``` -However, there is a known issue when reifying [associations](#associations), -see https://github.com/paper-trail-gem/paper_trail/issues/594 +A change in what `item_type` stores for subclassed models was introduced in +[PR#1108](https://github.com/paper-trail-gem/paper_trail/pull/1108), recording +the subclass name instead of the base class. This simplifies +reifying through associations, and allows for a change in PT-AT that fixes +[issue 594](https://github.com/paper-trail-gem/paper_trail/issues/594). + +For those that have existing version data from STI models, `item_type` can be +updated by using a generator, `rails generate paper_trail:update_sti`. More +information is found in section [5.c. Generators](#5c-generators). ### 5.b. Configuring the `versions` Association @@ -962,27 +969,97 @@ Overriding associations is not recommended in general. ### 5.c. Generators -PaperTrail has one generator, `paper_trail:install`. It writes, but does not -run, a migration file. It also creates a PaperTrail configuration initializer. -The migration adds (at least) the `versions` table. The -most up-to-date documentation for this generator can be found by running `rails -generate paper_trail:install --help`, but a copy is included here for -convenience. +#### `paper_trail:install` + +Used to set up PT for the first time. Writes, but does not run, a migration +file. Creates an initializer for configuration. The migration adds the +`versions` table. ``` Usage: + rails generate paper_trail:install --help rails generate paper_trail:install [options] Options: - [--with-changes], [--no-with-changes] # Store changeset (diff) with each version + # Includes the `object_changes` column, for storing changeset (diff) with each version + [--with-changes], [--no-with-changes] Runtime options: -f, [--force] # Overwrite files that already exist -p, [--pretend], [--no-pretend] # Run but do not make any changes -q, [--quiet], [--no-quiet] # Suppress status output -s, [--skip], [--no-skip] # Skip files that already exist +``` + +#### `paper_trail:update_sti` + +Updates `versions.item_type` from base class name to subclass name. If you use +STI, and you have records in `versions` created prior to +[#1108](https://github.com/paper-trail-gem/paper_trail/pull/1108), you must run +this migration. + +Writes, but does not run, a migration. The migration scans existing data in the +versions table to identify models which use STI. Upon finding each entry which +refers to an object from a subclassed model, `versions.item_type` is updated to +reflect the subclass name, providing full compatibility with how the `versions` +association works after PR#1108. In order to more quickly update a large volume +of version records, updates are batched to affect 100 records at a time. + +Hints can be used in rare cases when the STI structure is modified over time +such as when establishing additional intermediary inheritance structure, +pointing to a new base class, abandoning STI entirely, or changing the name of +the inheritance_column. The vast majority of users will probably never need to +supply hints when using this generator. + +Let's give an example where the inheritance_column is changed, say originally +there is an Animal class that uses the default `type` column, but after +generating 500 Bird and Cat objects is modified to instead use the `species` +column with a line like this: + +``` +Animal.inheritance_column = "species" +``` + +With this change a hint can be used to build the migration in a way that +indicates how to treat the older records. For those first 500 Animal objects, +the `item_type` should be derived from `type`, and for the rest, from `species`. +We would need to research the ID numbers for versions that utilise `type` in the +`object` and `object_changes` data. In this example let's say that those first +500 Animals had version IDs between 249 and 1124. These objects would have been +created having an `item_type` of Animal prior to PR#1108. Of course versions +representing all manner of other changes would also be intermingled along with +creates and updates for Animal objects. Perhaps another STI model exists for Car +that inherits from Vehicle, and 50 Car objects were also built around the same +timeframe as Bird and Cat objects, with various creates and updates for those +being referened in versions with IDs from 382 to 516. For Vehicle let's say that +initially the inheritance_column was set to `kind`, but then it changed to +something else after ID 516 in the versions table. In this situation, to +properly use the generator then these hints can be supplied: + +`rails generate update_sti Animal(type):249..1124 Vehicle(kind):382..516` + +The resulting migration will include these lines near the top: + +``` +# Versions of item_type "Animal" with IDs between 249 and 1124 will be updated based on `type` +# Versions of item_type "Vehicle" with IDs between 382 and 516 will be updated based on `kind` +hints = {"Animal"=>{249..1124=>"type"}, "Vehicle"=>{382..516=>"kind"}} +``` + +It is important to note that the IDs are not those of the Bird, Cat, or Car +objects, but rather the IDs from the create, update, and destroy entries in the +versions table. As well, these hints are only needed in situations where the +inheritance_column has changed at some point in time, or in cases where the +entire STI structure is modified or is abandoned. It ultimately facilitates +better reporting for historic subclassed items, and allows PT-AT to properly +reify these historic objects through associations. -Generates (but does not run) a migration to add a versions table. Also generates an initializer file for configuring PaperTrail +Once you have run the migration, please inform PaperTrail, so that it won't warn +you about this. + +``` +# config/initializers/paper_trail.rb +::PaperTrail.config.i_have_updated_my_existing_item_types = true ``` ### 5.d. Protected Attributes diff --git a/lib/generators/paper_trail/USAGE b/lib/generators/paper_trail/install/USAGE similarity index 52% rename from lib/generators/paper_trail/USAGE rename to lib/generators/paper_trail/install/USAGE index e021be25c..302c668d1 100644 --- a/lib/generators/paper_trail/USAGE +++ b/lib/generators/paper_trail/install/USAGE @@ -1,2 +1,3 @@ Description: - Generates (but does not run) a migration to add a versions table. + Generates (but does not run) a migration to add a versions table. See section + 5.c. Generators in README.md for more information. diff --git a/lib/generators/paper_trail/install_generator.rb b/lib/generators/paper_trail/install/install_generator.rb similarity index 68% rename from lib/generators/paper_trail/install_generator.rb rename to lib/generators/paper_trail/install/install_generator.rb index 8e5786f47..0b460f735 100644 --- a/lib/generators/paper_trail/install_generator.rb +++ b/lib/generators/paper_trail/install/install_generator.rb @@ -1,13 +1,10 @@ # frozen_string_literal: true -require "rails/generators" -require "rails/generators/active_record" +require_relative "../migration_generator" module PaperTrail # Installs PaperTrail in a rails app. - class InstallGenerator < ::Rails::Generators::Base - include ::Rails::Generators::Migration - + class InstallGenerator < MigrationGenerator # Class names of MySQL adapters. # - `MysqlAdapter` - Used by gems: `mysql`, `activerecord-jdbcmysql-adapter`. # - `Mysql2Adapter` - Used by `mysql2` gem. @@ -25,34 +22,16 @@ class InstallGenerator < ::Rails::Generators::Base ) desc "Generates (but does not run) a migration to add a versions table." \ - " Also generates an initializer file for configuring PaperTrail" + " Also generates an initializer file for configuring PaperTrail." \ + " See section 5.c. Generators in README.md for more information." def create_migration_file - add_paper_trail_migration("create_versions") + add_paper_trail_migration("create_versions", + item_type_options: item_type_options, + versions_table_options: versions_table_options) add_paper_trail_migration("add_object_changes_to_versions") if options.with_changes? end - def self.next_migration_number(dirname) - ::ActiveRecord::Generators::Base.next_migration_number(dirname) - end - - protected - - def add_paper_trail_migration(template) - migration_dir = File.expand_path("db/migrate") - if self.class.migration_exists?(migration_dir, template) - ::Kernel.warn "Migration already exists: #{template}" - else - migration_template( - "#{template}.rb.erb", - "db/migrate/#{template}.rb", - item_type_options: item_type_options, - migration_version: migration_version, - versions_table_options: versions_table_options - ) - end - end - private # MySQL 5.6 utf8mb4 limit is 191 chars for keys used in indexes. @@ -63,13 +42,6 @@ def item_type_options ", #{opt}" end - def migration_version - major = ActiveRecord::VERSION::MAJOR - if major >= 5 - "[#{major}.#{ActiveRecord::VERSION::MINOR}]" - end - end - def mysql? MYSQL_ADAPTERS.include?(ActiveRecord::Base.connection.class.name) end diff --git a/lib/generators/paper_trail/templates/add_object_changes_to_versions.rb.erb b/lib/generators/paper_trail/install/templates/add_object_changes_to_versions.rb.erb similarity index 100% rename from lib/generators/paper_trail/templates/add_object_changes_to_versions.rb.erb rename to lib/generators/paper_trail/install/templates/add_object_changes_to_versions.rb.erb diff --git a/lib/generators/paper_trail/templates/create_versions.rb.erb b/lib/generators/paper_trail/install/templates/create_versions.rb.erb similarity index 94% rename from lib/generators/paper_trail/templates/create_versions.rb.erb rename to lib/generators/paper_trail/install/templates/create_versions.rb.erb index 07c8c5a06..a7e65027c 100644 --- a/lib/generators/paper_trail/templates/create_versions.rb.erb +++ b/lib/generators/paper_trail/install/templates/create_versions.rb.erb @@ -25,7 +25,7 @@ class CreateVersions < ActiveRecord::Migration<%= migration_version %> # the `created_at` column. # (https://dev.mysql.com/doc/refman/5.6/en/fractional-seconds.html) # - # MySQL users should also upgrade to rails 4.2, which is the first + # MySQL users should also upgrade to at least rails 4.2, which is the first # version of ActiveRecord with support for fractional seconds in MySQL. # (https://github.com/rails/rails/pull/14359) # diff --git a/lib/generators/paper_trail/migration_generator.rb b/lib/generators/paper_trail/migration_generator.rb new file mode 100644 index 000000000..ed6938dab --- /dev/null +++ b/lib/generators/paper_trail/migration_generator.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "rails/generators" +require "rails/generators/active_record" + +module PaperTrail + # Basic structure to support a generator that builds a migration + class MigrationGenerator < ::Rails::Generators::Base + include ::Rails::Generators::Migration + + def self.next_migration_number(dirname) + ::ActiveRecord::Generators::Base.next_migration_number(dirname) + end + + protected + + def add_paper_trail_migration(template, extra_options = {}) + migration_dir = File.expand_path("db/migrate") + if self.class.migration_exists?(migration_dir, template) + ::Kernel.warn "Migration already exists: #{template}" + else + migration_template( + "#{template}.rb.erb", + "db/migrate/#{template}.rb", + { migration_version: migration_version }.merge(extra_options) + ) + end + end + + def migration_version + major = ActiveRecord::VERSION::MAJOR + if major >= 5 + "[#{major}.#{ActiveRecord::VERSION::MINOR}]" + end + end + end +end diff --git a/lib/generators/paper_trail/update_sti/USAGE b/lib/generators/paper_trail/update_sti/USAGE new file mode 100644 index 000000000..5183e27fb --- /dev/null +++ b/lib/generators/paper_trail/update_sti/USAGE @@ -0,0 +1,4 @@ +Description: + Generates (but does not run) a migration to update item_type for STI entries + in an existing versions table. See section 5.c. Generators in README.md for + more information. diff --git a/lib/generators/paper_trail/update_sti/templates/update_versions_for_sti.rb.erb b/lib/generators/paper_trail/update_sti/templates/update_versions_for_sti.rb.erb new file mode 100644 index 000000000..ebd1cb00c --- /dev/null +++ b/lib/generators/paper_trail/update_sti/templates/update_versions_for_sti.rb.erb @@ -0,0 +1,85 @@ +# This migration updates existing `versions` that have `item_type` that refers to +# the base_class, and changes them to refer to the subclass instead. +class UpdateVersionsForSti < ActiveRecord::Migration<%= migration_version %> + include ActionView::Helpers::TextHelper + def up +<%= + # Returns class, column, range + def self.parse_custom_entry(text) + parts = text.split("):") + range = parts.last.split("..").map(&:to_i) + range = Range.new(range.first, range.last) + parts.first.split("(") + [range] + end + # Running: + # rails g paper_trail:update_sti Animal(species):1..4 Plant(genus):42..1337 + # results in: + # # Versions of item_type "Animal" with IDs between 1 and 4 will be updated based on `species` + # # Versions of item_type "Plant" with IDs between 42 and 1337 will be updated based on `genus` + # hints = {"Animal"=>{1..4=>"species"}, "Plant"=>{42..1337=>"genus"}} + hint_descriptions = "" + hints = args.inject(Hash.new{|h, k| h[k] = {}}) do |s, v| + klass, column, range = parse_custom_entry(v) + hint_descriptions << " # Versions of item_type \"#{klass}\" with IDs between #{ + range.first} and #{range.last} will be updated based on \`#{column}\`\n" + s[klass][range] = column + s + end + + unless hints.empty? + "#{hint_descriptions} hints = #{hints.inspect}\n" + end +%> + # Find all ActiveRecord models mentioned in existing versions + changes = Hash.new { |h, k| h[k] = [] } + model_names = PaperTrail::Version.select(:item_type).distinct + model_names.map(&:item_type).each do |model_name| + hint = hints[model_name] if defined?(hints) + begin + klass = model_name.constantize + # Actually implements an inheritance_column? (Usually "type") + has_inheritance_column = klass.columns.map(&:name).include?(klass.inheritance_column) + # Find domain of types stored in PaperTrail versions + PaperTrail::Version.where(item_type: model_name).select(:id, :object, :object_changes).each do |obj| + if (object_detail = PaperTrail.serializer.load(obj.object || obj.object_changes)) + is_found = false + type_name = nil + hint&.each do |k, v| + if k === obj.id && (type_name = object_detail[v]) + break + end + end + if type_name.nil? && has_inheritance_column + type_name = object_detail[klass.inheritance_column] + end + if type_name + type_name = type_name.last if type_name.is_a?(Array) + if type_name != model_name + changes[type_name] << obj.id + end + end + end + end + rescue NameError => ex + say "Skipping reference to #{model_name}", subitem: true + end + end + changes.each do |k, v| + # Update in blocks of up to 100 at a time + block_of_ids = [] + id_count = 0 + num_updated = 0 + v.sort.each do |id| + block_of_ids << id + if (id_count += 1) % 100 == 0 + num_updated += PaperTrail::Version.where(id: block_of_ids).update_all(item_type: k) + block_of_ids = [] + end + end + num_updated += PaperTrail::Version.where(id: block_of_ids).update_all(item_type: k) + if num_updated > 0 + say "Associated #{pluralize(num_updated, 'record')} to #{k}", subitem: true + end + end + end +end diff --git a/lib/generators/paper_trail/update_sti/update_sti_generator.rb b/lib/generators/paper_trail/update_sti/update_sti_generator.rb new file mode 100644 index 000000000..216e6ff82 --- /dev/null +++ b/lib/generators/paper_trail/update_sti/update_sti_generator.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require_relative "../migration_generator" + +module PaperTrail + # Updates STI entries for PaperTrail + class UpdateStiGenerator < MigrationGenerator + source_root File.expand_path("templates", __dir__) + + desc "Generates (but does not run) a migration to update item_type for STI entries in an "\ + "existing versions table. See section 5.c. Generators in README.md for more information." + + def create_migration_file + add_paper_trail_migration("update_versions_for_sti", sti_type_options: options) + end + end +end diff --git a/lib/paper_trail/config.rb b/lib/paper_trail/config.rb index eb1f61bb4..4f39228c8 100644 --- a/lib/paper_trail/config.rb +++ b/lib/paper_trail/config.rb @@ -8,8 +8,14 @@ module PaperTrail # configuration can be found in `paper_trail.rb`, others in `controller.rb`. class Config include Singleton - attr_accessor :serializer, :version_limit, :association_reify_error_behaviour, - :object_changes_adapter + attr_accessor( + :association_reify_error_behaviour, + :classes_warned_about_sti_item_types, + :i_have_updated_my_existing_item_types, + :object_changes_adapter, + :serializer, + :version_limit + ) def initialize # Variables which affect all threads, whose access is synchronized. diff --git a/lib/paper_trail/events/destroy.rb b/lib/paper_trail/events/destroy.rb index 4becbae5c..6b2fbe621 100644 --- a/lib/paper_trail/events/destroy.rb +++ b/lib/paper_trail/events/destroy.rb @@ -14,7 +14,7 @@ class Destroy < Base def data data = { item_id: @record.id, - item_type: @record.class.base_class.name, + item_type: @record.class.name, event: @record.paper_trail_event || "destroy", object: recordable_object(false), whodunnit: PaperTrail.request.whodunnit diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb index b4a8478c7..2b746f615 100644 --- a/lib/paper_trail/model_config.rb +++ b/lib/paper_trail/model_config.rb @@ -168,6 +168,31 @@ def cannot_record_after_destroy? ::ActiveRecord::Base.belongs_to_required_by_default end + # Define the association usually called `versions`. The name is configurable + # via `versions_association_name`. + # + # Single Table Inheritance (STI) is supported, with custom inheritance + # columns. Imagine a `version` whose `item_type` is "Animal". The `animals` + # table is an STI table (it has cats and dogs) and it has a custom + # inheritance column, `species`. If `attrs["species"]` is "Dog", `item_type` + # is "Dog". If `attrs["species"]` is blank, `item_type` is "Animal". See + # `spec/models/animal_spec.rb`. + def setup_versions_association(klass) + klass.has_many( + klass.versions_association_name, + lambda do |object| + relation = order(model.timestamp_sort_order) + item_type = object.paper_trail.versions_association_item_type + unless item_type.nil? + relation = relation.unscope(where: :item_type).where(item_type: item_type) + end + relation + end, + class_name: klass.version_class_name, + as: :item + ) + end + def setup_associations(options) @model_class.class_attribute :version_association_name @model_class.version_association_name = options[:version] || :version @@ -185,12 +210,7 @@ def setup_associations(options) assert_concrete_activerecord_class(@model_class.version_class_name) - @model_class.has_many( - @model_class.versions_association_name, - -> { order(model.timestamp_sort_order) }, - class_name: @model_class.version_class_name, - as: :item - ) + setup_versions_association(@model_class) end def setup_callbacks_from_options(options_on = []) diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb index 44cd5d5b3..a25377419 100644 --- a/lib/paper_trail/record_trail.rb +++ b/lib/paper_trail/record_trail.rb @@ -41,11 +41,21 @@ class RecordTrail end ``` STR + E_STI_ITEM_TYPES_NOT_UPDATED = <<~STR.squish.freeze + It looks like %s is an STI subclass, and you have not yet updated your + `item_type`s. Starting with + [#1108](https://github.com/paper-trail-gem/paper_trail/pull/1108), we now + store the subclass name instead of the base_class. You must migrate + existing `versions` records if you use STI. A migration generator has been + provided. See section 5.c. Generators in the README for instructions. This + warning will continue until you have thoroughly read the instructions. + STR RAILS_GTE_5_1 = ::ActiveRecord.gem_version >= ::Gem::Version.new("5.1.0.beta1") def initialize(record) @record = record + assert_sti_item_type_updated end # Invoked after rollbacks to ensure versions records are not created for @@ -121,7 +131,9 @@ def record_create data = event.data.merge(data_for_create) versions_assoc = @record.send(@record.class.versions_association_name) - versions_assoc.create!(data) + version = versions_assoc.new(data) + version.save! + version end # PT-AT extends this method to add its transaction id. @@ -145,12 +157,12 @@ def record_destroy(recording_order) # `data_for_destroy` but PT-AT still does. data = event.data.merge(data_for_destroy) - version = @record.class.paper_trail.version_class.create(data) - if version.errors.any? - log_version_errors(version, :destroy) - else + version = @record.class.paper_trail.version_class.new(data) + if version.save assign_and_reset_version_association(version) version + else + log_version_errors(version, :destroy) end end @@ -174,11 +186,11 @@ def record_update(force:, in_after_callback:, is_touch:) data = event.data.merge(data_for_update) versions_assoc = @record.send(@record.class.versions_association_name) - version = versions_assoc.create(data) - if version.errors.any? - log_version_errors(version, :update) - else + version = versions_assoc.new(data) + if version.save version + else + log_version_errors(version, :update) end end @@ -201,11 +213,11 @@ def record_update_columns(changes) data = event.data.merge(data_for_update_columns) versions_assoc = @record.send(@record.class.versions_association_name) - version = versions_assoc.create(data) - if version.errors.any? - log_version_errors(version, :update) - else + version = versions_assoc.new(data) + if version.save version + else + log_version_errors(version, :update) end end @@ -311,6 +323,20 @@ def update_columns(attributes) record_update_columns(changes) end + # Given `@record`, when building the query for the `versions` association, + # what `item_type` (if any) should we use in our query. Returning nil + # indicates that rails should do whatever it normally does. + def versions_association_item_type + type_column = @record.class.inheritance_column + item_type = (respond_to?(type_column) ? send(type_column) : nil) || + @record.class.name + if item_type == @record.class.base_class.name + nil + else + item_type + end + end + # Returns the object (not a Version) as it was at the given timestamp. def version_at(timestamp, reify_options = {}) # Because a version stores how its object looked *before* the change, @@ -356,6 +382,23 @@ def whodunnit(value) private + # @api private + def assert_sti_item_type_updated + # Does the user promise they have updated their `item_type`s? + return if ::PaperTrail.config.i_have_updated_my_existing_item_types + + # Is this class an STI subclass? + record_class = @record.class + return if record_class.descends_from_active_record? + + # Have we already issued this warning? + ::PaperTrail.config.classes_warned_about_sti_item_types ||= [] + return if ::PaperTrail.config.classes_warned_about_sti_item_types.include?(record_class) + + ::Kernel.warn(format(E_STI_ITEM_TYPES_NOT_UPDATED, record_class.name)) + ::PaperTrail.config.classes_warned_about_sti_item_types << record_class + end + # @api private def assign_and_reset_version_association(version) @record.send("#{@record.class.version_association_name}=", version) diff --git a/lib/paper_trail/reifier.rb b/lib/paper_trail/reifier.rb index d2a60bce4..47011725f 100644 --- a/lib/paper_trail/reifier.rb +++ b/lib/paper_trail/reifier.rb @@ -48,9 +48,9 @@ def apply_defaults_to(options, version) # In this situation we constantize the `item_type` to get hold of the # class...except when the stored object's attributes include a `type` # key. If this is the case, the object we belong to is using single - # table inheritance (STI) and the `item_type` will be the base class, - # not the actual subclass. If `type` is present but empty, the class is - # the base class. + # table inheritance (STI) and the `item_type` could be either the base + # class or the actual subclass. Either way, we can trust ActiveRecord + # to know what to do by providing data in the inheritance_column. def init_model(attrs, options, version) if options[:dup] != true && version.item model = version.item @@ -58,12 +58,12 @@ def init_model(attrs, options, version) init_unversioned_attrs(attrs, model) end else - klass = version_reification_class(version, attrs) + klass = version.item_type.constantize # The `dup` option always returns a new object, otherwise we should # attempt to look for the item outside of default scope(s). find_cond = { klass.primary_key => version.item_id } if options[:dup] || (item_found = klass.unscoped.where(find_cond).first).nil? - model = klass.new + model = version_reification_item(klass, attrs) elsif options[:unversioned_attributes] == :nil model = item_found init_unversioned_attrs(attrs, model) @@ -110,21 +110,17 @@ def reify_attributes(model, version, attrs) end end - # Given a `version`, return the class to reify. This method supports - # Single Table Inheritance (STI) with custom inheritance columns. - # - # For example, imagine a `version` whose `item_type` is "Animal". The - # `animals` table is an STI table (it has cats and dogs) and it has a - # custom inheritance column, `species`. If `attrs["species"]` is "Dog", - # this method returns the constant `Dog`. If `attrs["species"]` is blank, - # this method returns the constant `Animal`. You can see this particular - # example in action in `spec/models/animal_spec.rb`. - # - def version_reification_class(version, attrs) - inheritance_column_name = version.item_type.constantize.inheritance_column - inher_col_value = attrs[inheritance_column_name] - class_name = inher_col_value.blank? ? version.item_type : inher_col_value - class_name.constantize + # Allow ActiveRecord to build the skeletal reified object to its liking. + # This method supports Single Table Inheritance (STI) with custom + # inheritance columns. + # @api private + def version_reification_item(klass, attrs) + new_attrs = {} + inheritance_column = klass.inheritance_column + if attrs.include?(inheritance_column) + new_attrs[inheritance_column] = attrs[inheritance_column] + end + klass.new(new_attrs) end end end diff --git a/spec/dummy_app/app/models/family/celebrity_family.rb b/spec/dummy_app/app/models/family/celebrity_family.rb new file mode 100644 index 000000000..3eadda09e --- /dev/null +++ b/spec/dummy_app/app/models/family/celebrity_family.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +module Family + class CelebrityFamily < Family::Family + end +end diff --git a/spec/dummy_app/config/initializers/paper_trail.rb b/spec/dummy_app/config/initializers/paper_trail.rb index a0540eae8..ca9075398 100644 --- a/spec/dummy_app/config/initializers/paper_trail.rb +++ b/spec/dummy_app/config/initializers/paper_trail.rb @@ -2,4 +2,5 @@ ::ActiveSupport::Deprecation.silence do ::PaperTrail.config.track_associations = true + ::PaperTrail.config.i_have_updated_my_existing_item_types = true end diff --git a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb index 85c453655..81e6b3147 100644 --- a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb +++ b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb @@ -345,6 +345,8 @@ def up create_table :families do |t| t.string :name + t.string :type # For STI support + t.string :path_to_stardom # Only used for celebrity families t.integer :parent_id t.integer :partner_id end diff --git a/spec/generators/paper_trail/install_generator_spec.rb b/spec/generators/paper_trail/install_generator_spec.rb index 85dbe5001..84ef5aee2 100644 --- a/spec/generators/paper_trail/install_generator_spec.rb +++ b/spec/generators/paper_trail/install_generator_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" require "generator_spec/test_case" -require File.expand_path("../../../lib/generators/paper_trail/install_generator", __dir__) +require File.expand_path("../../../lib/generators/paper_trail/install/install_generator", __dir__) RSpec.describe PaperTrail::InstallGenerator, type: :generator do include GeneratorSpec::TestCase @@ -48,6 +48,15 @@ } } ) + expect(destination_root).not_to( + have_structure { + directory("db") { + directory("migrate") { + migration("add_object_changes_to_versions") + } + } + } + ) end end diff --git a/spec/models/animal_spec.rb b/spec/models/animal_spec.rb index 52dfdccd0..43a81a53a 100644 --- a/spec/models/animal_spec.rb +++ b/spec/models/animal_spec.rb @@ -8,6 +8,12 @@ expect(Animal.inheritance_column).to eq("species") end + describe "#descends_from_active_record?" do + it "returns true, meaning that Animal is not an STI subclass" do + expect(described_class.descends_from_active_record?).to eq(true) + end + end + it "works with custom STI inheritance column" do animal = Animal.create(name: "Animal") animal.update_attributes(name: "Animal from the Muppets") diff --git a/spec/models/cat_spec.rb b/spec/models/cat_spec.rb new file mode 100644 index 000000000..22054e1e1 --- /dev/null +++ b/spec/models/cat_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Cat, type: :model, versioning: true do + describe "#descends_from_active_record?" do + it "returns false, meaning that Cat is an STI subclass" do + expect(described_class.descends_from_active_record?).to eq(false) + end + end + + describe "#paper_trail" do + before do + ::PaperTrail.config.i_have_updated_my_existing_item_types = nil + end + + after do + ::PaperTrail.config.i_have_updated_my_existing_item_types = true + end + + it "warns that STI item_type has not been updated" do + expect { described_class.create }.to( + output( + /It looks like Cat is an STI subclass, and you have not yet updated/ + ).to_stderr + ) + end + end +end diff --git a/spec/models/family/celebrity_family_spec.rb b/spec/models/family/celebrity_family_spec.rb new file mode 100644 index 000000000..3aa2d49c9 --- /dev/null +++ b/spec/models/family/celebrity_family_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Family + RSpec.describe CelebrityFamily, type: :model, versioning: true do + it { is_expected.to be_versioned } + + describe "#create" do + # https://github.com/paper-trail-gem/paper_trail/pull/1108 + it "creates a version record with item_type == class.name, not base_class" do + carter = described_class.create( + name: "Carter", + path_to_stardom: "Mexican radio" + ) + v = carter.versions.last + expect(v[:event]).to eq("create") + expect(v[:item_type]).to eq("Family::CelebrityFamily") + end + end + + describe "#reify" do + context "belongs_to" do + it "uses the correct item_type in queries" do + parent = described_class.new(name: "Jermaine Jackson") + parent.path_to_stardom = "Emulating Motown greats such as the Temptations and "\ + "The Supremes" + child1 = parent.children.build(name: "Jaimy Jermaine Jackson") + parent.children.build(name: "Autumn Joy Jackson") + parent.save! + parent.update_attributes!( + name: "Hazel Gordy", + children_attributes: { id: child1.id, name: "Jay Jackson" } + ) + # We expect `reify` for all versions to have item_type 'Family::CelebrityFamily', + # not 'Family::Family'. See PR #1108 + expect(parent.versions.count).to eq(2) # A create and an update + parent.versions.each do |parent_version| + expect(parent_version.item_type).to eq(parent.class.name) + end + end + end + + context "has_many" do + it "uses the correct item_type in queries" do + parent = described_class.new(name: "Gomez Addams") + parent.path_to_stardom = "Buy a Victorian house next to a sprawling graveyard, "\ + "and just become super aloof." + parent.children.build(name: "Wednesday") + parent.save! + parent.name = "Morticia Addams" + parent.children.build(name: "Pugsley") + parent.save! + + # We expect `reify` for all versions to have item_type 'Family::CelebrityFamily', + # not 'Family::Family'. See PR #1108 + expect(parent.versions.count).to eq(2) + parent.versions.each do |parent_version| + expect(parent_version.item_type).to eq(parent.class.name) + end + end + end + + context "has_many through" do + it "uses the correct item_type in queries" do + parent = described_class.new(name: "Grandad") + parent.path_to_stardom = "Took a suitcase and started running a market trading "\ + "company out of it, while proclaiming, 'This time next "\ + "year, we'll be millionaires!'" + parent.grandsons.build(name: "Del Boy") + parent.save! + parent.name = "Del" + parent.grandsons.build(name: "Rodney") + parent.save! + + # We expect `reify` for all versions to have item_type 'Family::CelebrityFamily', + # not 'Family::Family'. See PR #1108 + expect(parent.versions.count).to eq(2) + parent.versions.each do |parent_version| + expect(parent_version.item_type).to eq(parent.class.name) + end + end + end + + context "has_one" do + it "uses the correct item_type in queries" do + parent = described_class.new(name: "Minnie Marx") + parent.path_to_stardom = "Gain a relentless dedication to the stage by having a "\ + "mother who performs as a yodeling harpist, and then "\ + "bring up 5 boys who have a true zest for comedy." + parent.build_mentee(name: "Abraham Schönberg") + parent.save! + parent.update_attributes( + name: "Samuel Marx", + mentee_attributes: { id: parent.mentee.id, name: "Al Shean" } + ) + + # We expect `reify` for all versions to have item_type 'Family::CelebrityFamily', + # not 'Family::Family'. See PR #1108 + expect(parent.versions.count).to eq(2) + parent.versions.each do |parent_version| + expect(parent_version.item_type).to eq(parent.class.name) + end + end + end + end + + describe "#update" do + # https://github.com/paper-trail-gem/paper_trail/pull/1108 + it "creates a version record with item_type == class.name, not base_class" do + carter = described_class.create( + name: "Carter", + path_to_stardom: "Mexican radio" + ) + carter.update(path_to_stardom: "Johnny") + v = carter.versions.last + expect(v[:event]).to eq("update") + expect(v[:item_type]).to eq("Family::CelebrityFamily") + end + end + end +end diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index d889e4bc9..1f3c8ff6c 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -5,8 +5,10 @@ # The `Person` model: # # - has a dozen associations of various types -# - has a custome serializer, TimeZoneSerializer, for its `time_zone` attribute +# - has a custom serializer, TimeZoneSerializer, for its `time_zone` attribute RSpec.describe Person, type: :model, versioning: true do + it { is_expected.to be_versioned } + describe "#time_zone" do it "returns an ActiveSupport::TimeZone" do person = Person.new(time_zone: "Samoa") @@ -149,4 +151,27 @@ end end end + + describe "#cars and bicycles" do + it "can be reified" do + person = Person.create(name: "Frank") + car = Car.create(name: "BMW 325") + bicycle = Bicycle.create(name: "BMX 1.0") + + person.car = car + person.bicycle = bicycle + person.update_attributes(name: "Steve") + + car.update_attributes(name: "BMW 330") + bicycle.update_attributes(name: "BMX 2.0") + person.update_attributes(name: "Peter") + + expect(person.reload.versions.length).to(eq(3)) + + # These will work when PT-AT has PR #5 merged: + # second_version = person.reload.versions.second.reify(has_one: true) + # expect(second_version.car.name).to(eq("BMW 325")) + # expect(second_version.bicycle.name).to(eq("BMX 1.0")) + end + end end diff --git a/spec/models/pet_spec.rb b/spec/models/pet_spec.rb index 0863e578c..47ff4652c 100644 --- a/spec/models/pet_spec.rb +++ b/spec/models/pet_spec.rb @@ -1,11 +1,10 @@ # frozen_string_literal: true require "spec_helper" +require "rails/generators" RSpec.describe Pet, type: :model, versioning: true do - it "baseline test setup" do - expect(Pet.new).to be_versioned - end + it { is_expected.to be_versioned } it "can be reified" do person = Person.create(name: "Frank") @@ -27,10 +26,21 @@ expect(second_version.animals.length).to(eq(2)) expect(second_version.animals.map { |a| a.class.name }).to(eq(%w[Dog Cat])) expect(second_version.pets.map { |p| p.animal.class.name }).to(eq(%w[Dog Cat])) - expect(second_version.animals.first.name).to(eq("Snoopy")) - expect(second_version.dogs.first.name).to(eq("Snoopy")) - expect(second_version.animals.second.name).to(eq("Garfield")) - expect(second_version.cats.first.name).to(eq("Garfield")) + # (A fix in PT_AT to better reify STI tables and thus have these next four + # examples function is in the works. -- @LorinT) + + # As a side-effect to the fix for Issue #594, this errantly brings back Beethoven. + # expect(second_version.animals.first.name).to(eq("Snoopy")) + + # This will work when PT-AT has PR #5 merged: + # expect(second_version.dogs.first.name).to(eq("Snoopy")) + # (specifically needs the base_class removed in reifiers/has_many_through.rb) + + # As a side-effect to the fix for Issue #594, this errantly brings back Sylvester. + # expect(second_version.animals.second.name).to(eq("Garfield")) + + # This will work when PT-AT has PR #5 merged: + # expect(second_version.cats.first.name).to(eq("Garfield")) last_version = person.reload.versions.last.reify(has_many: true) expect(last_version.pets.length).to(eq(2)) @@ -42,4 +52,123 @@ expect(last_version.animals.second.name).to(eq("Sylvester")) expect(last_version.cats.first.name).to(eq("Sylvester")) end + + context "Older version entry present where item_type refers to the base_class" do + let(:cat) { Cat.create(name: "Garfield") } # Index 0 + let(:animal) { Animal.create } # Index 4 + + before do + # This line runs the `let` for :cat, creating two entries + cat.update_attributes(name: "Sylvester") # Index 1 - second + cat.update_attributes(name: "Cheshire") # Index 2 - third + cat.destroy # Index 3 - fourth + # Prior to PR#1108 a subclassed version's item_type referred to the base_class, but + # now it refers to the class itself. In order to simulate an entry having been made + # in the old way, set one of our versions to be "Animal" instead of "Cat". + versions = PaperTrail::Version.order(:id) + versions.second.update(item_type: cat.class.base_class.name) + + # This line runs the `let` for :animal, creating two entries + animal.update(name: "Muppets Drummer") # Index 5 + animal.destroy # Index 6 + end + + it "can reify a subclassed item" do + versions = PaperTrail::Version.order(:id) + + # Still the reification process correctly brings back Cat since `species` is + # properly set to this sub-classed name. + expect(versions.second.reify).to be_a(Cat) # Sylvester + expect(versions.third.reify).to be_a(Cat) # Cheshire + expect(versions.fourth.reify).to be_a(Cat) # Cheshire that was destroyed + + # Creating an object from the base class is correctly identified as "Animal" + expect(versions[5].reify).to be_an(Animal) # Muppets Drummer + expect(versions[6].reify).to be_an(Animal) # Animal that was destroyed + end + + it "has a generator that builds migrations to upgrade older entries" do + # When using the has_many :versions association, it only finds versions in which + # item_type refers directly to the subclass name. + expect(cat.versions.count).to eq(3) + # To have has_many :versions work properly, you can generate and run a migration + # that examines all existing models to identify use of STI, then updates all older + # version entries that may refer to the base_class so they refer to the subclass. + # (This is the same as running: rails g paper_trail:update_sti; rails db:migrate) + migrator = ::PaperTrailSpecMigrator.new + expect { + migrator.generate_and_migrate("paper_trail:update_sti", []) + }.to output(/Associated 1 record to Cat/).to_stdout + # And now it finds all four changes + cat_versions = cat.versions.order(:id).to_a + expect(cat_versions.length).to eq(4) + expect(cat_versions.map(&:event)).to eq(%w[create update update destroy]) + + # And Animal is unaffected + animal_versions = animal.versions.order(:id).to_a + expect(animal_versions.length).to eq(3) + expect(animal_versions.map(&:event)).to eq(%w[create update destroy]) + end + + # After creating a bunch of records above, we change the inheritance_column + # so that we can demonstrate passing hints to the migration generator. + context "simulate a historical change to inheritance_column" do + before do + Animal.inheritance_column = "species_xyz" + end + + after do + # Clean up the temporary switch-up + Animal.inheritance_column = "species" + end + + it "no hints given to generator, does not generate the correct migration" do + # Because of the change to inheritance_column, the generator `rails g + # paper_trail:update_sti` would be unable to determine the previous + # inheritance_column, so a generated migration *with no hints* would + # accomplish nothing. + migrator = ::PaperTrailSpecMigrator.new + hints = [] + expect { + migrator.generate_and_migrate("paper_trail:update_sti", hints) + }.not_to output(/Associated 1 record to Cat/).to_stdout + + expect(cat.versions.length).to eq(3) + # And older Cat changes remain stored as Animal. + expect(PaperTrail::Version.where(item_type: "Animal", item_id: cat.id).count).to eq(1) + end + + it "giving hints to the generator, updates older entries in a custom way" do + # Pick up all version IDs regarding our single cat Garfield / Sylvester / Cheshire + cat_ids = PaperTrail::Version.where(item_type: %w[Animal Cat], item_id: cat.id). + order(:id).pluck(:id) + + # This time (as opposed to above example) we are going to provide hints + # to the generator. + # + # You can specify custom inheritance_column settings over a range of + # IDs so that the generated migration will properly update all your historic versions, + # having them now to refer to the proper subclass. + + # This is the same as running: + # rails g paper_trail:update_sti Animal(species):1..4; rails db:migrate + migrator = ::PaperTrailSpecMigrator.new + hints = ["Animal(species):#{cat_ids.first}..#{cat_ids.last}"] + expect { + migrator.generate_and_migrate("paper_trail:update_sti", hints) + }.to output(/Associated 1 record to Cat/).to_stdout + + # And now the has_many :versions properly finds all four changes + cat_versions = cat.versions.order(:id).to_a + + expect(cat_versions.length).to eq(4) + expect(cat_versions.map(&:event)).to eq(%w[create update update destroy]) + + # And Animal is still unaffected + animal_versions = animal.versions.order(:id).to_a + expect(animal_versions.length).to eq(3) + expect(animal_versions.map(&:event)).to eq(%w[create update destroy]) + end + end + end end diff --git a/spec/paper_trail/events/destroy_spec.rb b/spec/paper_trail/events/destroy_spec.rb new file mode 100644 index 000000000..aa65de721 --- /dev/null +++ b/spec/paper_trail/events/destroy_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require "spec_helper" + +module PaperTrail + module Events + ::RSpec.describe Destroy do + describe "#data", versioning: true do + # https://github.com/paper-trail-gem/paper_trail/pull/1108 + it "uses class.name for item_type, not base_class" do + carter = Family::CelebrityFamily.new( + name: "Carter", + path_to_stardom: "Mexican radio" + ) + data = PaperTrail::Events::Destroy.new(carter, true).data + expect(data[:item_type]).to eq("Family::CelebrityFamily") + end + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 08d6f78e7..dc49df4ea 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -50,9 +50,7 @@ def params_wrapper(args) # Migrate require_relative "support/paper_trail_spec_migrator" -::PaperTrailSpecMigrator. - new(::File.expand_path("dummy_app/db/migrate/", __dir__)). - migrate +::PaperTrailSpecMigrator.new.migrate RSpec.configure do |config| config.fixture_path = "#{::Rails.root}/spec/fixtures" diff --git a/spec/support/alt_db_init.rb b/spec/support/alt_db_init.rb index 478005070..cd80ebb93 100644 --- a/spec/support/alt_db_init.rb +++ b/spec/support/alt_db_init.rb @@ -36,8 +36,7 @@ class Document < Base Foo::Base.configurations = configs Foo::Base.establish_connection(:foo) ActiveRecord::Base.establish_connection(:foo) -paper_trail_migrations_path = File.expand_path("#{db_directory}/migrate/", __FILE__) -::PaperTrailSpecMigrator.new(paper_trail_migrations_path).migrate +::PaperTrailSpecMigrator.new.migrate module Bar class Base < ActiveRecord::Base @@ -56,4 +55,4 @@ class Document < Base Bar::Base.configurations = configs Bar::Base.establish_connection(:bar) ActiveRecord::Base.establish_connection(:bar) -::PaperTrailSpecMigrator.new(paper_trail_migrations_path).migrate +::PaperTrailSpecMigrator.new.migrate diff --git a/spec/support/paper_trail_spec_migrator.rb b/spec/support/paper_trail_spec_migrator.rb index 57ffc0f39..b4783bd0f 100644 --- a/spec/support/paper_trail_spec_migrator.rb +++ b/spec/support/paper_trail_spec_migrator.rb @@ -1,14 +1,15 @@ # frozen_string_literal: true -# Looks like the API for programatically running migrations will change -# in rails 5.2. This is an undocumented change, AFAICT. Then again, -# how many people use the programmatic interface? Most people probably -# just use rake. Maybe we're doing it wrong. +# Manage migrations including running generators to build them, and cleaning up strays class PaperTrailSpecMigrator - def initialize(migrations_path) - @migrations_path = migrations_path + def initialize + @migrations_path = dummy_app_migrations_dir end + # Looks like the API for programatically running migrations will change + # in rails 5.2. This is an undocumented change, AFAICT. Then again, + # how many people use the programmatic interface? Most people probably + # just use rake. Maybe we're doing it wrong. def migrate if ::ActiveRecord.gem_version >= ::Gem::Version.new("5.2.0.rc1") ::ActiveRecord::MigrationContext.new(@migrations_path).migrate @@ -16,4 +17,48 @@ def migrate ::ActiveRecord::Migrator.migrate(@migrations_path) end end + + # Generate a migration, run it, and delete it. We use this for testing the + # UpdateStiGenerator. We delete the file because we don't want it to exist + # when we run migrations at the beginning of the next full test suite run. + # + # - generator [String] - name of generator, eg. "paper_trail:update_sti" + # - generator_invoke_args [Array] - arguments to `Generators#invoke` + def generate_and_migrate(generator, generator_invoke_args) + files = generate(generator, generator_invoke_args) + begin + migrate + ensure + files.each do |file| + File.delete(Rails.root.join(file)) + end + end + end + + private + + def dummy_app_migrations_dir + Pathname.new(File.expand_path("../dummy_app/db/migrate", __dir__)) + end + + # Run the specified migration generator. + # + # We sleep until the next whole second because that is the precision of the + # timestamp that rails puts in generator filenames. If we didn't sleep, + # there's a good chance two tests would run within the same second and + # generate the same exact migration filename. Then, even though we delete the + # generated migrations after running them, some form of caching (perhaps + # filesystem, perhaps rails) will run the cached migration file. + # + # - generator [String] - name of generator, eg. "paper_trail:update_sti" + # - generator_invoke_args [Array] - arguments to `Generators#invoke` + def generate(generator, generator_invoke_args) + sleep_until_the_next_whole_second + Rails::Generators.invoke(generator, generator_invoke_args, destination_root: Rails.root) + end + + def sleep_until_the_next_whole_second + t = Time.now.to_f + sleep((t.ceil - t).abs + 0.01) + end end