Merge pull request #790 from flippercloud/mysql-fix

ActiveRecord adapter: Fix migration error for MySQL

Author: bkeepers

.github/workflows/ci.yml

@@ -13,6 +13,18 @@ jobs:
           --health-interval 10s
           --health-timeout 5s
           --health-retries 5
+      postgres:
+        image: postgres:13
+        ports:
+          - 5432:5432
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
     strategy:
       matrix:
         ruby: ['2.6', '2.7', '3.0', '3.1', '3.2']
@@ -41,7 +53,13 @@ jobs:
       REDIS_URL: redis://localhost:6379/0
       CI: true
       RAILS_VERSION: ${{ matrix.rails }}
+      MYSQL_USER: root
+      MYSQL_PASSWORD: root
+      POSTGRES_USER: postgres
+      POSTGRES_PASSWORD: postgres
     steps:
+      - name: Set up MySQL
+        run: sudo /etc/init.d/mysql start
       - name: Setup memcached
         uses: KeisukeYamashita/memcached-actions@v1
       - name: Start MongoDB

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).

Gemfile

@@ -25,6 +25,8 @@ gem 'benchmark-ips'
 gem 'stackprof-webnav'
 gem 'flamegraph'
 gem 'climate_control'
+gem 'mysql2'
+gem 'pg'
 
 group(:guard) do
   gem 'guard'

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
@@ -12,10 +12,10 @@ class CreateFlipperTables < ActiveRecord::Migration<%= migration_version %>
       t.text :value
       t.timestamps null: false
     end
-    add_index :flipper_gates, [:feature_key, :key, :value], unique: true
+    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,18 @@
+# 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
+
+    if index_exists? :flipper_gates, [:feature_key, :key, :value]
+      remove_index :flipper_gates, [:feature_key, :key, :value]
+    end
+    change_column :flipper_gates, :value, :text
+    add_index :flipper_gates, [:feature_key, :key, :value], unique: true, length: { value: 255 }
+  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