From f92cfbc23ddf8dc62055e4d489c4132be2a3d9ee Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Mon, 10 Apr 2017 11:10:15 -0400 Subject: [PATCH 1/2] Inherit from the new versioned migration class e.g. `ActiveRecord::Migration[5.1]` Fixes #949 Following the example of the devise gem, use erb. In addition, add the `.erb` file extension to fix syntax highlighting in editor. Deletes create_versions_spec.rb because the file is no longer valid ruby so those tests will have to be re-written. We can either generate a valid ruby file and continue to use similar assertions, or we can convert those features to use erb, which will have the benefit of cleaner generated files. --- CHANGELOG.md | 3 +- .../paper_trail/install_generator.rb | 13 ++++- ... => add_object_changes_to_versions.rb.erb} | 0 ..._transaction_id_column_to_versions.rb.erb} | 0 ....rb => create_version_associations.rb.erb} | 0 ...ate_versions.rb => create_versions.rb.erb} | 2 +- spec/generators/install_generator_spec.rb | 11 +++- .../templates/create_versions_spec.rb | 51 ------------------- 8 files changed, 25 insertions(+), 55 deletions(-) rename lib/generators/paper_trail/templates/{add_object_changes_to_versions.rb => add_object_changes_to_versions.rb.erb} (100%) rename lib/generators/paper_trail/templates/{add_transaction_id_column_to_versions.rb => add_transaction_id_column_to_versions.rb.erb} (100%) rename lib/generators/paper_trail/templates/{create_version_associations.rb => create_version_associations.rb.erb} (100%) rename lib/generators/paper_trail/templates/{create_versions.rb => create_versions.rb.erb} (97%) delete mode 100644 spec/generators/paper_trail/templates/create_versions_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index bc86dd8d..81f840e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Fixed -- None +- [#949](https://github.com/airblade/paper_trail/issues/949) - Inherit from the + new versioned migration class, e.g. `ActiveRecord::Migration[5.1]` ## 7.0.0 (2017-04-01) diff --git a/lib/generators/paper_trail/install_generator.rb b/lib/generators/paper_trail/install_generator.rb index 3ef0fdc0..9f06014c 100644 --- a/lib/generators/paper_trail/install_generator.rb +++ b/lib/generators/paper_trail/install_generator.rb @@ -50,7 +50,18 @@ module PaperTrail if self.class.migration_exists?(migration_dir, template) ::Kernel.warn "Migration already exists: #{template}" else - migration_template "#{template}.rb", "db/migrate/#{template}.rb" + migration_template( + "#{template}.rb.erb", + "db/migrate/#{template}.rb", + migration_version: migration_version + ) + end + end + + def migration_version + major = ActiveRecord::VERSION::MAJOR + if major >= 5 + "[#{major}.#{ActiveRecord::VERSION::MINOR}]" end end end diff --git a/lib/generators/paper_trail/templates/add_object_changes_to_versions.rb b/lib/generators/paper_trail/templates/add_object_changes_to_versions.rb.erb similarity index 100% rename from lib/generators/paper_trail/templates/add_object_changes_to_versions.rb rename to lib/generators/paper_trail/templates/add_object_changes_to_versions.rb.erb diff --git a/lib/generators/paper_trail/templates/add_transaction_id_column_to_versions.rb b/lib/generators/paper_trail/templates/add_transaction_id_column_to_versions.rb.erb similarity index 100% rename from lib/generators/paper_trail/templates/add_transaction_id_column_to_versions.rb rename to lib/generators/paper_trail/templates/add_transaction_id_column_to_versions.rb.erb diff --git a/lib/generators/paper_trail/templates/create_version_associations.rb b/lib/generators/paper_trail/templates/create_version_associations.rb.erb similarity index 100% rename from lib/generators/paper_trail/templates/create_version_associations.rb rename to lib/generators/paper_trail/templates/create_version_associations.rb.erb diff --git a/lib/generators/paper_trail/templates/create_versions.rb b/lib/generators/paper_trail/templates/create_versions.rb.erb similarity index 97% rename from lib/generators/paper_trail/templates/create_versions.rb rename to lib/generators/paper_trail/templates/create_versions.rb.erb index 900cb1a7..7b5beed0 100644 --- a/lib/generators/paper_trail/templates/create_versions.rb +++ b/lib/generators/paper_trail/templates/create_versions.rb.erb @@ -1,6 +1,6 @@ # This migration creates the `versions` table, the only schema PT requires. # All other migrations PT provides are optional. -class CreateVersions < ActiveRecord::Migration +class CreateVersions < ActiveRecord::Migration<%= migration_version %> # Class names of MySQL adapters. # - `MysqlAdapter` - Used by gems: `mysql`, `activerecord-jdbcmysql-adapter`. # - `Mysql2Adapter` - Used by `mysql2` gem. diff --git a/spec/generators/install_generator_spec.rb b/spec/generators/install_generator_spec.rb index ab0e1338..0194a274 100644 --- a/spec/generators/install_generator_spec.rb +++ b/spec/generators/install_generator_spec.rb @@ -15,12 +15,21 @@ RSpec.describe PaperTrail::InstallGenerator, type: :generator do end it "generates a migration for creating the 'versions' table" do + expected_parent_class = lambda { + old_school = "ActiveRecord::Migration" + ar_version = ActiveRecord::VERSION + if ar_version::MAJOR >= 5 + format("%s[%d.%d]", old_school, ar_version::MAJOR, ar_version::MINOR) + else + old_school + end + }.call expect(destination_root).to( have_structure { directory("db") { directory("migrate") { migration("create_versions") { - contains "class CreateVersions" + contains("class CreateVersions < " + expected_parent_class) contains "def change" contains "create_table :versions" } diff --git a/spec/generators/paper_trail/templates/create_versions_spec.rb b/spec/generators/paper_trail/templates/create_versions_spec.rb deleted file mode 100644 index 7e9b118f..00000000 --- a/spec/generators/paper_trail/templates/create_versions_spec.rb +++ /dev/null @@ -1,51 +0,0 @@ -require "rails_helper" -require "generators/paper_trail/templates/create_versions" - -RSpec.describe CreateVersions do - describe "#change", verify_stubs: false do - let(:migration) { described_class.new } - - before do - allow(migration).to receive(:add_index) - allow(migration).to receive(:create_table) - end - - it "creates the versions table" do - migration.change - expect(migration).to have_received(:create_table) do |arg1| - expect(arg1).to eq(:versions) - end - end - - case ENV["DB"] - when "mysql" - it "uses InnoDB engine" do - migration.change - expect(migration).to have_received(:create_table) do |_, arg2| - expect(arg2[:options]).to match(/ENGINE=InnoDB/) - end - end - - it "uses utf8mb4 character set" do - migration.change - expect(migration).to have_received(:create_table) do |_, arg2| - expect(arg2[:options]).to match(/DEFAULT CHARSET=utf8mb4/) - end - end - - it "uses utf8mb4_col collation" do - migration.change - expect(migration).to have_received(:create_table) do |_, arg2| - expect(arg2[:options]).to match(/COLLATE=utf8mb4_general_ci/) - end - end - else - it "passes an empty options hash to create_table" do - migration.change - expect(migration).to have_received(:create_table) do |_, arg2| - expect(arg2).to eq({}) - end - end - end - end -end From ddde7857c5b74d50c5a66a467fed7e23bda297c9 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Mon, 10 Apr 2017 11:35:18 -0400 Subject: [PATCH 2/2] Generate cleaner migrations --- CHANGELOG.md | 2 +- .../paper_trail/install_generator.rb | 49 ++++++++++++++++++- .../templates/create_versions.rb.erb | 48 +----------------- spec/generators/install_generator_spec.rb | 9 +++- 4 files changed, 59 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81f840e9..8149d169 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Added -- None +- Generate cleaner migrations for databases other than MySQL ### Fixed diff --git a/lib/generators/paper_trail/install_generator.rb b/lib/generators/paper_trail/install_generator.rb index 9f06014c..8f6e56fa 100644 --- a/lib/generators/paper_trail/install_generator.rb +++ b/lib/generators/paper_trail/install_generator.rb @@ -6,6 +6,14 @@ module PaperTrail class InstallGenerator < ::Rails::Generators::Base include ::Rails::Generators::Migration + # 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", __FILE__) class_option( :with_changes, @@ -53,16 +61,55 @@ module PaperTrail migration_template( "#{template}.rb.erb", "db/migrate/#{template}.rb", - migration_version: migration_version + 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. + # See https://github.com/airblade/paper_trail/issues/651 + def item_type_options + opt = { null: false } + opt[:limit] = 191 if mysql? + ", #{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 + + # 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 diff --git a/lib/generators/paper_trail/templates/create_versions.rb.erb b/lib/generators/paper_trail/templates/create_versions.rb.erb index 7b5beed0..07c8c5a0 100644 --- a/lib/generators/paper_trail/templates/create_versions.rb.erb +++ b/lib/generators/paper_trail/templates/create_versions.rb.erb @@ -1,13 +1,6 @@ # This migration creates the `versions` table, the only schema PT requires. # All other migrations PT provides are optional. class CreateVersions < ActiveRecord::Migration<%= migration_version %> - # 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 # The largest text column available in all supported RDBMS is # 1024^3 - 1 bytes, roughly one gibibyte. We specify a size @@ -16,8 +9,8 @@ class CreateVersions < ActiveRecord::Migration<%= migration_version %> TEXT_BYTES = 1_073_741_823 def change - create_table :versions, versions_table_options do |t| - t.string :item_type, item_type_options + create_table :versions<%= versions_table_options %> do |t| + t.string :item_type<%= item_type_options %> t.integer :item_id, null: false t.string :event, null: false t.string :whodunnit @@ -40,41 +33,4 @@ class CreateVersions < ActiveRecord::Migration<%= migration_version %> end add_index :versions, %i(item_type item_id) end - - private - - # MySQL 5.6 utf8mb4 limit is 191 chars for keys used in indexes. - # See https://github.com/airblade/paper_trail/issues/651 - def item_type_options - opt = { null: false } - opt[:limit] = 191 if mysql? - opt - end - - def mysql? - MYSQL_ADAPTERS.include?(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 diff --git a/spec/generators/install_generator_spec.rb b/spec/generators/install_generator_spec.rb index 0194a274..e25bc397 100644 --- a/spec/generators/install_generator_spec.rb +++ b/spec/generators/install_generator_spec.rb @@ -24,6 +24,13 @@ RSpec.describe PaperTrail::InstallGenerator, type: :generator do old_school end }.call + expected_create_table_options = lambda { + if described_class::MYSQL_ADAPTERS.include?(ActiveRecord::Base.connection.class.name) + ', { options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci" }' + else + "" + end + }.call expect(destination_root).to( have_structure { directory("db") { @@ -31,7 +38,7 @@ RSpec.describe PaperTrail::InstallGenerator, type: :generator do migration("create_versions") { contains("class CreateVersions < " + expected_parent_class) contains "def change" - contains "create_table :versions" + contains "create_table :versions#{expected_create_table_options}" } } }