Fix for issue paper-trail-gem#594, reifying sub-classed models that u…

…se STI (paper-trail-gem#1108)

See the changes to the changelog and readme for details.

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

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

lib/generators/paper_trail/USAGE → 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.

lib/generators/paper_trail/install_generator.rb → 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

lib/generators/paper_trail/templates/create_versions.rb.erb → 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)
       #

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

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.

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

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

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.