Merge branch 'update-generator' into mysql-fix

* update-generator:
  Update changelog and warning message
  Add tests for update migrations
  Add `generate flipper:update` to add migrations
  Fix running of generator test, update test

Changelog.md

@@ -12,6 +12,10 @@ All notable changes to this project will be documented in this file.
   undefined local variable or method `default_strict_value' for Flipper::Engine:Class
   ```
 
+### Additions/Changes
+
+- Added `$ rails generate flipper:update` to generate necessary schema migrations (https://github.com/flippercloud/flipper/pull/787)
+
 ## v1.1.1
 
 ### Bug Fixes
@@ -40,12 +44,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)
+- 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)
   ```
-  $ rails generate migration change_flipper_gates_value_to_text
-  ```
-  ```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).

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.

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, length: { value: 255 }
   end
 
-  def self.down
+  def down
     drop_table :flipper_gates
     drop_table :flipper_features
   end

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

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

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

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

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

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