Generator to update historic item_subtype entries (#1144)

* Generator to update historic item_subtype entries

* Simpler migrations for Foo and Bar test databases

* Rubocop indentation

lib/generators/paper_trail/USAGE

@@ -1,2 +0,0 @@
-Description:
-  Generates (but does not run) a migration to add a versions table.

lib/generators/paper_trail/install/USAGE

@@ -0,0 +1,3 @@
+Description:
+  Generates (but does not run) a migration to add a versions table. Also generates an initializer
+  file for configuring PaperTrail. See section 5.c. Generators in README.md for more information.

lib/generators/paper_trail/install/install_generator.rb

@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+require_relative "../migration_generator"
+
+module PaperTrail
+  # Installs PaperTrail in a rails app.
+  class InstallGenerator < MigrationGenerator
+    # Class names of MySQL adapters.
+    # - `MysqlAdapter` - Used by gems: `mysql`, `activerecord-jdbcmysql-adapter`.
+    # - `Mysql2Adapter` - Used by `mysql2` gem.
+    MYSQL_ADAPTERS = [
+      "ActiveRecord::ConnectionAdapters::MysqlAdapter",
+      "ActiveRecord::ConnectionAdapters::Mysql2Adapter"
+    ].freeze
+
+    source_root File.expand_path("templates", __dir__)
+    class_option(
+      :with_changes,
+      type: :boolean,
+      default: false,
+      desc: "Store changeset (diff) with each version"
+    )
+
+    desc "Generates (but does not run) a migration to add a versions table." \
+         "  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",
+        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
+
+    private
+
+    # MySQL 5.6 utf8mb4 limit is 191 chars for keys used in indexes.
+    # See https://github.com/paper-trail-gem/paper_trail/issues/651
+    def item_type_options
+      opt = { null: false }
+      opt[:limit] = 191 if mysql?
+      ", #{opt}"
+    end
+
+    def mysql?
+      MYSQL_ADAPTERS.include?(ActiveRecord::Base.connection.class.name)
+    end
+
+    # Even modern versions of MySQL still use `latin1` as the default character
+    # encoding. Many users are not aware of this, and run into trouble when they
+    # try to use PaperTrail in apps that otherwise tend to use UTF-8. Postgres, by
+    # comparison, uses UTF-8 except in the unusual case where the OS is configured
+    # with a custom locale.
+    #
+    # - https://dev.mysql.com/doc/refman/5.7/en/charset-applications.html
+    # - http://www.postgresql.org/docs/9.4/static/multibyte.html
+    #
+    # Furthermore, MySQL's original implementation of UTF-8 was flawed, and had
+    # to be fixed later by introducing a new charset, `utf8mb4`.
+    #
+    # - https://mathiasbynens.be/notes/mysql-utf8mb4
+    # - https://dev.mysql.com/doc/refman/5.5/en/charset-unicode-utf8mb4.html
+    #
+    def versions_table_options
+      if mysql?
+        ', { options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci" }'
+      else
+        ""
+      end
+    end
+  end
+end

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_item_subtype/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_item_subtype/templates/update_versions_for_item_subtype.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 UpdateVersionsForItemSubtype < 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_item_subtype 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, item_subtype: nil).select(:id, :object, :object_changes).each do |obj|
+          if (object_detail = PaperTrail.serializer.load(obj.object || obj.object_changes))
+            is_found = false
+            subtype_name = nil
+            hint&.each do |k, v|
+              if k === obj.id && (subtype_name = object_detail[v])
+                break
+              end
+            end
+            if subtype_name.nil? && has_inheritance_column
+              subtype_name = object_detail[klass.inheritance_column]
+            end
+            if subtype_name
+              subtype_name = subtype_name.last if subtype_name.is_a?(Array)
+              if subtype_name != model_name
+                changes[subtype_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_subtype: k)
+          block_of_ids = []
+        end
+      end
+      num_updated += PaperTrail::Version.where(id: block_of_ids).update_all(item_subtype: k)
+      if num_updated > 0
+        say "Associated #{pluralize(num_updated, 'record')} to #{k}", subitem: true
+      end
+    end
+  end
+end

lib/generators/paper_trail/update_item_subtype/update_item_subtype_generator.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require_relative "../migration_generator"
+
+module PaperTrail
+  # Updates STI entries for PaperTrail
+  class UpdateItemSubtypeGenerator < MigrationGenerator
+    source_root File.expand_path("templates", __dir__)
+
+    desc "Generates (but does not run) a migration to update item_subtype for STI entries in an "\
+      "existing versions table."
+
+    def create_migration_file
+      add_paper_trail_migration("update_versions_for_item_subtype", sti_type_options: options)
+    end
+  end
+end

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 @@ RSpec.describe PaperTrail::InstallGenerator, type: :generator do
           }
         }
       )
+      expect(destination_root).not_to(
+        have_structure {
+          directory("db") {
+            directory("migrate") {
+              migration("add_object_changes_to_versions")
+            }
+          }
+        }
+      )
     end
   end
 

spec/models/pet_spec.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
+require "rails/generators"
 
 RSpec.describe Pet, type: :model, versioning: true do
   it "baseline test setup" do
@@ -42,4 +43,121 @@ RSpec.describe Pet, type: :model, versioning: true do
     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#1143 a subclassed version's item_subtype would be nil.  In order to simulate
+      # an entry having been made in the old way, set one of the item_subtype entries to be nil
+      # instead of "Cat".
+      versions = PaperTrail::Version.order(:id)
+      versions.second.update(item_subtype: nil)
+
+      # 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
+      # Only newer versions have item_subtype that refers directly to the subclass name.
+      expect(PaperTrail::Version.where(item_subtype: "Cat").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_item_subtype", [])
+      }.to output(/Associated 1 record to Cat/).to_stdout
+
+      # And now it finds all four changes
+      cat_versions = PaperTrail::Version.where(item_subtype: "Cat").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 = PaperTrail::Version.where(item_subtype: "Animal").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_item_subtype", hints)
+        }.not_to output(/Associated 1 record to Cat/).to_stdout
+
+        expect(PaperTrail::Version.where(item_subtype: "Cat").count).to eq(3)
+        # And older Cat changes remain as nil.
+        expect(PaperTrail::Version.where(item_subtype: nil, 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: "Animal", 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_item_subtype", 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

spec/spec_helper.rb

@@ -50,9 +50,7 @@ require "ffaker"
 
 # 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"

spec/support/alt_db_init.rb

@@ -40,8 +40,7 @@ end
 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
@@ -60,4 +59,4 @@ end
 Bar::Base.configurations = configs
 Bar::Base.establish_connection(:bar)
 ActiveRecord::Base.establish_connection(:bar)
-::PaperTrailSpecMigrator.new(paper_trail_migrations_path).migrate
+::PaperTrailSpecMigrator.new.migrate

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 @@ class PaperTrailSpecMigrator
       ::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