diff --git a/Changelog.md b/Changelog.md index e542f74d4..ac5f6509d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,12 @@ All notable changes to this project will be documented in this file. +## v1.1.2 + +### Additions/Changes + +- Added `$ rails generate flipper:update && rails db:migrate` to generate necessary schema migrations (https://github.com/flippercloud/flipper/pull/787) + ## v1.1.1 ### Bug Fixes @@ -30,12 +36,9 @@ All notable changes to this project will be documented in this file. ``` - Handle deprecation of Rack::File in Rack 3.1 (https://github.com/flippercloud/flipper/pull/773). - Human readable actor names in flipper-ui (https://github.com/flippercloud/flipper/pull/737). -- Expressions are now available and considered "alpha". They are not yet documented, but you can see examples in [examples/expressions.rb](examples/expressions.rb). Those using the `flipper-active_record` adapter will want to migrate the database so it can store JSON expressions (https://github.com/flippercloud/flipper/pull/692) - ``` - $ rails generate migration change_flipper_gates_value_to_text +- Expressions are now available and considered "alpha". They are not yet documented, but you can see examples in [examples/expressions.rb](examples/expressions.rb). Those using the `flipper-active_record` adapter will want to [migrate the database](See https://github.com/flippercloud/flipper/issues/557) so it can store JSON expressions (https://github.com/flippercloud/flipper/pull/692) ``` - ```ruby - change_column :flipper_gates, :value, :text + $ rails generate flipper:update && rails db:migrate ``` - Allow head requests to api (https://github.com/flippercloud/flipper/pull/759) - Cloud Telemetry alpha (https://github.com/flippercloud/flipper/pull/775). diff --git a/lib/flipper/adapters/active_record.rb b/lib/flipper/adapters/active_record.rb index 33ef56dbc..d9fb9cbcc 100644 --- a/lib/flipper/adapters/active_record.rb +++ b/lib/flipper/adapters/active_record.rb @@ -34,7 +34,7 @@ class Gate < Model VALUE_TO_TEXT_WARNING = <<-EOS Your database needs migrated to use the latest Flipper features. - See https://github.com/flippercloud/flipper/issues/557 + Run `rails generate flipper:update` and `rails db:migrate`. EOS # Public: Initialize a new ActiveRecord adapter instance. diff --git a/lib/generators/flipper/templates/migration.erb b/lib/generators/flipper/templates/migration.erb index f51e80ffc..1bdbc7406 100644 --- a/lib/generators/flipper/templates/migration.erb +++ b/lib/generators/flipper/templates/migration.erb @@ -1,5 +1,5 @@ class CreateFlipperTables < ActiveRecord::Migration<%= migration_version %> - def self.up + def up create_table :flipper_features do |t| t.string :key, null: false t.timestamps null: false @@ -15,7 +15,7 @@ class CreateFlipperTables < ActiveRecord::Migration<%= migration_version %> add_index :flipper_gates, [:feature_key, :key, :value], unique: true end - def self.down + def down drop_table :flipper_gates drop_table :flipper_features end diff --git a/lib/generators/flipper/templates/update/migrations/01_create_flipper_tables.rb.erb b/lib/generators/flipper/templates/update/migrations/01_create_flipper_tables.rb.erb new file mode 100644 index 000000000..6ee99bd22 --- /dev/null +++ b/lib/generators/flipper/templates/update/migrations/01_create_flipper_tables.rb.erb @@ -0,0 +1,22 @@ +class CreateFlipperTables < ActiveRecord::Migration<%= migration_version %> + def up + create_table :flipper_features do |t| + t.string :key, null: false + t.timestamps null: false + end + add_index :flipper_features, :key, unique: true + + create_table :flipper_gates do |t| + t.string :feature_key, null: false + t.string :key, null: false + t.string :value + t.timestamps null: false + end + add_index :flipper_gates, [:feature_key, :key, :value], unique: true + end + + def down + drop_table :flipper_gates + drop_table :flipper_features + end +end diff --git a/lib/generators/flipper/templates/update/migrations/02_change_flipper_gates_value_to_text.rb.erb b/lib/generators/flipper/templates/update/migrations/02_change_flipper_gates_value_to_text.rb.erb new file mode 100644 index 000000000..bd8d9e7a2 --- /dev/null +++ b/lib/generators/flipper/templates/update/migrations/02_change_flipper_gates_value_to_text.rb.erb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class ChangeFlipperGatesValueToText < ActiveRecord::Migration<%= migration_version %> + def up + # Ensure this incremental update migration is idempotent + return unless connection.column_exists?(:flipper_gates, :value, :string) + + change_column :flipper_gates, :value, :text + end + + def down + change_column :flipper_gates, :value, :string + end +end diff --git a/lib/generators/flipper/update_generator.rb b/lib/generators/flipper/update_generator.rb new file mode 100644 index 000000000..f91498529 --- /dev/null +++ b/lib/generators/flipper/update_generator.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'rails/generators' +require 'rails/generators/active_record' + +module Flipper + module Generators + # + # Rails generator used for updating Flipper in a Rails application. + # Run it with +bin/rails g flipper:update+ in your console. + # + class UpdateGenerator < Rails::Generators::Base + include ActiveRecord::Generators::Migration + + TEMPLATES = File.join(File.dirname(__FILE__), 'templates/update') + source_paths << TEMPLATES + + # Generates incremental migration files unless they already exist. + # All migrations should be idempotent e.g. +add_index+ is guarded with +if_index_exists?+ + def update_migration_files + migration_templates = Dir.children(File.join(TEMPLATES, 'migrations')).sort + migration_templates.each do |template_file| + destination_file = template_file.match(/^\d*_(.*\.rb)/)[1] # 01_create_flipper_tables.rb.erb => create_flipper_tables.rb + migration_template "migrations/#{template_file}", File.join(db_migrate_path, destination_file), skip: true + end + end + + private + + def migration_version + "[#{ActiveRecord::VERSION::STRING.to_f}]" + end + end + end +end diff --git a/test_rails/generators/flipper/active_record_generator_test.rb b/test_rails/generators/flipper/active_record_generator_test.rb index aca969ec5..b861a20e3 100644 --- a/test_rails/generators/flipper/active_record_generator_test.rb +++ b/test_rails/generators/flipper/active_record_generator_test.rb @@ -1,5 +1,4 @@ -require 'active_record' -require 'rails/generators/test_case' +require 'helper' require 'generators/flipper/active_record_generator' class FlipperActiveRecordGeneratorTest < Rails::Generators::TestCase @@ -16,7 +15,7 @@ def test_generates_migration end assert_migration 'db/migrate/create_flipper_tables.rb', <<~MIGRATION class CreateFlipperTables < ActiveRecord::Migration#{migration_version} - def self.up + def up create_table :flipper_features do |t| t.string :key, null: false t.timestamps null: false @@ -26,13 +25,13 @@ def self.up create_table :flipper_gates do |t| t.string :feature_key, null: false t.string :key, null: false - t.string :value + t.text :value t.timestamps null: false end add_index :flipper_gates, [:feature_key, :key, :value], unique: true end - def self.down + def down drop_table :flipper_gates drop_table :flipper_features end diff --git a/test_rails/generators/flipper/update_generator_test.rb b/test_rails/generators/flipper/update_generator_test.rb new file mode 100644 index 000000000..a959ac9bf --- /dev/null +++ b/test_rails/generators/flipper/update_generator_test.rb @@ -0,0 +1,96 @@ +require "helper" +require "generators/flipper/update_generator" + +class BasicsGeneratorTest < Rails::Generators::TestCase + tests Flipper::Generators::UpdateGenerator + ROOT = File.expand_path("../../../../tmp/generators", __FILE__) + destination ROOT + setup :prepare_destination + + setup do + ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:') + end + + teardown do + ActiveRecord::Base.connection.close + end + + test "generates migrations" do + run_generator + + assert_migration "db/migrate/create_flipper_tables.rb" do |migration| + assert_method :up, migration do |up| + assert_match(/create_table :flipper_features/, up) + assert_match(/create_table :flipper_gates/, up) + end + + assert_method :down, migration do |down| + assert_match(/drop_table :flipper_features/, down) + assert_match(/drop_table :flipper_gates/, down) + end + end + + assert_migration "db/migrate/change_flipper_gates_value_to_text.rb" do |migration| + [:up, :down].each do |dir| + assert_method :up, migration do |method| + assert_match(/change_column/, method) + end + end + end + + require_migrations + + silence { CreateFlipperTables.migrate(:up) } + assert ActiveRecord::Base.connection.table_exists?(:flipper_features) + assert ActiveRecord::Base.connection.table_exists?(:flipper_gates) + + assert ActiveRecord::Base.connection.column_exists?(:flipper_gates, :value, :string) + silence { ChangeFlipperGatesValueToText.migrate(:up) } + assert ActiveRecord::Base.connection.column_exists?(:flipper_gates, :value, :text) + + silence { ChangeFlipperGatesValueToText.migrate(:down) } + assert ActiveRecord::Base.connection.column_exists?(:flipper_gates, :value, :string) + + silence { CreateFlipperTables.migrate(:down) } + refute ActiveRecord::Base.connection.table_exists?(:flipper_features) + refute ActiveRecord::Base.connection.table_exists?(:flipper_gates) + end + + test "ChangeFlipperGatesValueToText is a noop if value is already text" do + self.class.generator_class = Flipper::Generators::ActiveRecordGenerator + run_generator + + self.class.generator_class = Flipper::Generators::UpdateGenerator + run_generator + + assert_migration "db/migrate/create_flipper_tables.rb" do |migration| + assert_method :up, migration do |up| + assert_match /text :value/, up + end + end + + assert_migration "db/migrate/change_flipper_gates_value_to_text.rb" + + require_migrations + + silence { CreateFlipperTables.migrate(:up) } + assert ActiveRecord::Base.connection.column_exists?(:flipper_gates, :value, :text) + + assert_nothing_raised do + silence { ChangeFlipperGatesValueToText.migrate(:up) } + end + assert ActiveRecord::Base.connection.column_exists?(:flipper_gates, :value, :text) + end + + def require_migrations + # If these are not reloaded, then test order can cause failures + Object.send(:remove_const, :CreateFlipperTables) if defined?(::CreateFlipperTables) + Object.send(:remove_const, :ChangeFlipperGatesValueToText) if defined?(::ChangeFlipperGatesValueToText) + + Dir.glob("#{ROOT}/db/migrate/*.rb").each do |file| + assert_nothing_raised do + load file + end + end + end +end diff --git a/test_rails/helper.rb b/test_rails/helper.rb index 4589bdce2..10a98c326 100644 --- a/test_rails/helper.rb +++ b/test_rails/helper.rb @@ -1,6 +1,6 @@ require 'rubygems' -require 'bundler' -Bundler.setup(:default) +require 'bundler/setup' +require 'minitest/autorun' require 'rails' require 'rails/test_help' @@ -9,3 +9,20 @@ rescue NoMethodError # no biggie, means we are on older version of AS that doesn't have this option end + +def silence + # Store the original stderr and stdout in order to restore them later + original_stderr = $stderr + original_stdout = $stdout + + # Redirect stderr and stdout + output = $stderr = $stdout = StringIO.new + + yield + + $stderr = original_stderr + $stdout = original_stdout + + # Return output + output.string +end