Merge pull request #5 from anthonyalberto/patch-AR-migrations

Change strategy to patch sql statements issued to the db

.travis.yml

@@ -0,0 +1,12 @@
+language: ruby
+rvm:
+  - 2.0.0
+
+before_script:
+  - mysql -e 'create database mysql_online_migrations;'
+
+gemfile:
+  - gemfiles/rails3.gemfile
+  - gemfiles/rails4.gemfile
+
+script: bundle exec rspec spec

Gemfile.lock

@@ -24,7 +24,7 @@ GEM
     builder (3.0.4)
     coderay (1.0.9)
     diff-lcs (1.2.5)
-    i18n (0.6.5)
+    i18n (0.6.9)
     logger (1.2.8)
     method_source (0.8.2)
     multi_json (1.8.2)

gemfiles/rails3.gemfile

@@ -0,0 +1,7 @@
+source :rubygems
+gem "activerecord", "3.2.16"
+gem "activesupport", "3.2.16"
+gem "mysql2"
+gem "logger"
+gem "rspec"
+gem "pry"

gemfiles/rails4.gemfile

@@ -0,0 +1,7 @@
+source :rubygems
+gem "activerecord", "4.0.2"
+gem "activesupport", "4.0.2"
+gem "mysql2"
+gem "logger"
+gem "rspec"
+gem "pry"

lib/mysql_online_migrations.rb

@@ -1,33 +1,28 @@
 require 'active_record'
+require "active_record/migration"
 require "active_record/connection_adapters/mysql2_adapter"
+require "pry"
 
 %w(*.rb).each do |path|
   Dir["#{File.dirname(__FILE__)}/mysql_online_migrations/#{path}"].each { |f| require(f) }
 end
 
 module MysqlOnlineMigrations
-  include Indexes
-  include Columns
-
-  def self.included(base)
+  def self.prepended(base)
     ActiveRecord::Base.send(:class_attribute, :mysql_online_migrations, :instance_writer => false)
     ActiveRecord::Base.send("mysql_online_migrations=", true)
   end
 
-  def lock_statement(lock, with_comma = false)
-    return "" if lock == true
-    return "" unless perform_migrations_online?
-    puts "ONLINE MIGRATION"
-    "#{with_comma ? ', ' : ''} LOCK=NONE"
-  end
-
-  def extract_lock_from_options(options)
-    [options[:lock], options.except(:lock)]
+  def connection
+    @no_lock_adapter ||= ActiveRecord::ConnectionAdapters::Mysql2AdapterWithoutLock.new(super)
   end
 
-  def perform_migrations_online?
-    ActiveRecord::Base.mysql_online_migrations == true
+  def with_lock
+    original_value = ActiveRecord::Base.mysql_online_migrations
+    ActiveRecord::Base.mysql_online_migrations = false
+    yield
+    ActiveRecord::Base.mysql_online_migrations = original_value
   end
 end
 
-ActiveRecord::ConnectionAdapters::Mysql2Adapter.send(:include, MysqlOnlineMigrations)
+ActiveRecord::Migration.send(:prepend, MysqlOnlineMigrations)

lib/mysql_online_migrations/mysql2_adapter_without_lock.rb

@@ -0,0 +1,32 @@
+module ActiveRecord
+  module ConnectionAdapters
+    class Mysql2AdapterWithoutLock < Mysql2Adapter
+      OPTIMIZABLE_DDL_REGEX = /^(alter|create (unique )? ?index|drop index) /i
+      DDL_WITH_COMMA_REGEX = /^alter /i
+      DDL_WITH_LOCK_NONE_REGEX = / LOCK=NONE\s*$/i
+
+      def initialize(mysql2_adapter)
+        params = [:@connection, :@logger, :@connection_options, :@config].map do |sym|
+          mysql2_adapter.instance_variable_get(sym)
+        end
+        super(*params)
+      end
+
+      alias_method :original_execute, :execute
+      def execute(sql, name = nil)
+        if sql =~ OPTIMIZABLE_DDL_REGEX
+          sql = "#{sql} #{lock_none_statement(sql)}"
+          puts "EXECUTING #{sql}"
+        end
+        original_execute(sql, name)
+      end
+
+      def lock_none_statement(sql)
+        return "" unless ActiveRecord::Base.mysql_online_migrations
+        return "" if sql =~ DDL_WITH_LOCK_NONE_REGEX
+        comma_delimiter = (sql =~ DDL_WITH_COMMA_REGEX ? "," : "")
+        "#{comma_delimiter} LOCK=NONE"
+      end
+    end
+  end
+end

spec/lib/migration/column_spec.rb

@@ -0,0 +1,138 @@
+require "spec_helper"
+
+describe ActiveRecord::Migration do
+  let(:comma_before_lock_none) { true }
+  let(:migration_arguments_with_lock) { [] }
+
+  context "#add_column" do
+    let(:method_name) { :add_column }
+    let(:migration_arguments) do
+      [
+        [:testing, :foo2, :string],
+        [:testing, :foo2, :string, { limit: 20, null: false, default: 'def' }],
+        [:testing, :foo2, :decimal, { precision:3, scale: 2 }]
+      ]
+    end
+
+    it_behaves_like "a migration that adds LOCK=NONE when needed"
+    it_behaves_like "a migration that succeeds in MySQL"
+  end
+
+  context "#add_timestamps" do
+    let(:migration_arguments) do
+      [
+        [:testing2]
+      ]
+    end
+
+    let(:method_name) { :add_timestamps }
+
+    it_behaves_like "a migration that adds LOCK=NONE when needed"
+    it_behaves_like "a migration that succeeds in MySQL"
+  end
+
+  context "#remove_column" do
+    let(:migration_arguments) do
+      [
+        [:testing, :foo],
+        [:testing, :foo, :bar]
+      ]
+    end
+
+    let(:method_name) { :remove_column }
+
+    it_behaves_like "a migration that adds LOCK=NONE when needed"
+    it_behaves_like "a migration that succeeds in MySQL"
+  end
+
+  context "#remove_timestamps" do
+    let(:migration_arguments) do
+      [
+        [:testing]
+      ]
+    end
+
+    let(:method_name) { :remove_timestamps }
+
+    it_behaves_like "a migration that adds LOCK=NONE when needed"
+    it_behaves_like "a migration that succeeds in MySQL"
+  end
+
+  context "#change_column" do
+    let(:migration_arguments) do
+      # Unsupported with lock=none : change column type, change limit, set NOT NULL.
+      [
+        [:testing, :foo, :string, { default: 'def', limit: 100 }],
+        [:testing, :foo, :string, { null: true, limit: 100 }]
+      ]
+    end
+
+    let(:migration_arguments_with_lock) do
+      [
+        [:testing, :foo, :string, { limit: 200 }],
+        [:testing, :foo, :string, { default: 'def' }],
+        [:testing, :foo, :string, { null: false }],
+        [:testing, :foo, :string, { null: false, default: 'def', limit: 200 }],
+        [:testing, :foo, :string, { null: true }],
+        [:testing, :foo, :integer, { null: true, limit: 6 }],
+        [:testing, :foo, :integer, { null: true, limit: 1 }]
+      ]
+    end
+
+    let(:method_name) { :change_column }
+
+    it_behaves_like "a migration that adds LOCK=NONE when needed"
+    it_behaves_like "a migration that succeeds in MySQL"
+    it_behaves_like "a migration with a non-lockable statement"
+  end
+
+  context "#change_column_default" do
+    let(:migration_arguments) do
+      [
+        [:testing, :foo, 'def'],
+        [:testing, :foo, nil]
+      ]
+    end
+
+    let(:method_name) { :change_column_default }
+
+    it_behaves_like "a migration that adds LOCK=NONE when needed"
+    it_behaves_like "a migration that succeeds in MySQL"
+  end
+
+  context "#change_column_null" do
+    let(:migration_arguments) do
+      #change_column_null doesn't set DEFAULT in sql. It just issues an update statement before setting the NULL value if setting NULL to false
+      [
+        [:testing, :bam, true, nil],
+        [:testing, :bam, true, 'def']
+      ]
+    end
+
+    let(:migration_arguments_with_lock) do
+      [
+        [:testing, :bam, false, nil],
+        [:testing, :bam, false, 'def']
+      ]
+    end
+
+    let(:method_name) { :change_column_null }
+
+    it_behaves_like "a migration that adds LOCK=NONE when needed"
+    it_behaves_like "a migration that succeeds in MySQL"
+    it_behaves_like "a migration with a non-lockable statement"
+  end
+
+  context "#rename_column" do
+    let(:migration_arguments) do
+      [
+        [:testing, :foo, :foo2]
+      ]
+    end
+
+    let(:method_name) { :rename_column }
+
+    it_behaves_like "a migration that adds LOCK=NONE when needed"
+    it_behaves_like "a migration that succeeds in MySQL"
+  end
+end