Skip to content

Commit

Permalink
safer migration loading, some speedups
Browse files Browse the repository at this point in the history
we no longer run each migration in noop mode to get at the triggers
(using an AR::Migration monkey-patch)... this was uber-dangerous since
we only intercepted things that fell through to the connection (e.g.
create_table or execute), and not other stuff (e.g. Foo.update_all or
MyWebService.call). we now use ruby_parser to extract triggers (or in
the case of schema.rb, just a simple regex)

added some little regex checks to model and migration loading so that
we don't bother loading or parsing classes that have no triggers
  • Loading branch information
jenseng committed May 31, 2011
1 parent 6a27c58 commit d34d091
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 97 deletions.
7 changes: 5 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
source "http://rubygems.org"

gem "activerecord", ">=2.3.0"
gem 'ruby_parser', '2.0.6'
gem 'ruby2ruby', '1.2.5'
group :development do
gem "rspec", "~> 2.3.0"
gem "bundler", "~> 1.0.0"
gem "jeweler", "~> 1.5.2"
gem "jeweler", "~> 1.6.1"
gem "rcov", ">= 0"
gem 'mysql', '>= 2.8.1'
gem 'mysql2', '>= 0.2.7'
gem 'mysql2', '>= 0.2.7', '< 0.3'
gem 'pg', '>= 0.10.1'
gem 'sqlite3-ruby', '>= 1.3.2'
gem 'ruby-debug', '0.10.4'
end
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.1.10
0.1.11
53 changes: 19 additions & 34 deletions lib/hair_trigger.rb
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
require 'ostruct'
require 'hair_trigger/base'
require 'hair_trigger/builder'
require 'hair_trigger/migration_reader'
require 'hair_trigger/migrator'
require 'hair_trigger/migration'
require 'hair_trigger/adapter'
require 'hair_trigger/schema_dumper'
require 'hair_trigger/schema'

module HairTrigger
def self.current_triggers
# see what the models say there should be
canonical_triggers = []
Dir[model_path + '/*rb'].each do |model|
class_name = model.sub(/\A.*\/(.*?)\.rb\z/, '\1').camelize
next unless File.read(model) =~ /^\s*trigger[\.\(]/
begin
require model unless klass = Kernel.const_get(class_name) rescue nil
klass = Kernel.const_get(class_name)
rescue StandardError, LoadError
raise "unable to load #{class_name} and its trigger(s)" if File.read(model) =~ /^\s*trigger[\.\(]/
next
end
canonical_triggers += klass.triggers if klass < ActiveRecord::Base && klass.triggers
end
Expand All @@ -32,11 +31,6 @@ def self.current_migrations(options = {})
options[:skip_pending_migrations] = true
end

prev_verbose = ActiveRecord::Migration.verbose
ActiveRecord::Migration.verbose = false
ActiveRecord::Migration.extract_trigger_builders = true
ActiveRecord::Migration.extract_all_triggers = options[:include_manual_triggers] || false

# if we're in a db:schema:dump task (explict or kicked off by db:migrate),
# we evaluate the previous schema.rb (if it exists), and then all applied
# migrations in order (even ones older than schema.rb). this ensures we
Expand All @@ -47,27 +41,26 @@ def self.current_migrations(options = {})
# evaluate all migrations along with schema.rb, ordered by version
migrator = ActiveRecord::Migrator.new(:up, migration_path)
migrated = migrator.migrated rescue []
migrations = migrator.migrations.select{ |migration|
File.read(migration.filename) =~ /(create|drop)_trigger/ &&
(options[:skip_pending_migrations] ? migrated.include?(migration.version) : true)
}.each{ |migration|
migration.migrate(:up)
}

if options.has_key?(:previous_schema)
eval(options[:previous_schema]) if options[:previous_schema]
elsif File.exist?(schema_rb_path)
load(schema_rb_path)
migrations = []
migrator.migrations.each do |migration|
next if options[:skip_pending_migrations] && !migrated.include?(migration.version)
triggers = MigrationReader.get_triggers(migration, options)
migrations << [migration, triggers] unless triggers.empty?
end
if ActiveRecord::Schema.info && ActiveRecord::Schema.trigger_builders
migrations.unshift OpenStruct.new({:version => ActiveRecord::Schema.info[:version], :trigger_builders => ActiveRecord::Schema.trigger_builders})

if previous_schema = (options.has_key?(:previous_schema) ? options[:previous_schema] : File.exist?(schema_rb_path) && File.read(schema_rb_path))
base_triggers = MigrationReader.get_triggers(previous_schema, options)
unless base_triggers.empty?
version = (previous_schema =~ /ActiveRecord::Schema\.define\(:version => (\d+)\)/) && $1.to_i
migrations.unshift [OpenStruct.new({:version => version}), base_triggers]
end
end
migrations = migrations.sort_by(&:version) unless options[:schema_rb_first]

migrations = migrations.sort_by{|(migration, triggers)| migration.version} unless options[:schema_rb_first]

all_builders = []
migrations.each do |migration|
next unless migration.trigger_builders
migration.trigger_builders.each do |new_trigger|
migrations.each do |(migration, triggers)|
triggers.each do |new_trigger|
# if there is already a trigger with this name, delete it since we are
# either dropping it or replacing it
new_trigger.prepare!
Expand All @@ -77,11 +70,6 @@ def self.current_migrations(options = {})
end

all_builders

ensure
ActiveRecord::Migration.verbose = prev_verbose
ActiveRecord::Migration.extract_trigger_builders = false
ActiveRecord::Migration.extract_all_triggers = false
end

def self.migrations_current?
Expand Down Expand Up @@ -185,9 +173,6 @@ def adapter_name_for(adapter)
end

ActiveRecord::Base.send :extend, HairTrigger::Base
ActiveRecord::Migration.send :extend, HairTrigger::Migration
ActiveRecord::MigrationProxy.send :delegate, :trigger_builders, :to=>:migration
ActiveRecord::Migrator.send :extend, HairTrigger::Migrator
ActiveRecord::ConnectionAdapters::AbstractAdapter.class_eval { include HairTrigger::Adapter }
ActiveRecord::SchemaDumper.class_eval { include HairTrigger::SchemaDumper }
ActiveRecord::Schema.send :extend, HairTrigger::Schema
ActiveRecord::SchemaDumper.class_eval { include HairTrigger::SchemaDumper }
1 change: 1 addition & 0 deletions lib/hair_trigger/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ def trigger(name = nil, options = {})
name = nil
end
options[:compatibility] ||= ::HairTrigger::Builder::compatibility
options[:generated] = true
@triggers ||= []
trigger = ::HairTrigger::Builder.new(name, options)
@triggers << trigger
Expand Down
16 changes: 12 additions & 4 deletions lib/hair_trigger/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -212,17 +212,25 @@ def to_ruby(indent = '', always_generated = true)

def <=>(other)
ret = prepared_name <=> other.prepared_name
ret == 0 ? hash <=> other.hash : ret
return ret unless ret == 0
hash <=> other.hash
end

def ==(other)
components == other.components
end

def eql?(other)
return false unless other.is_a?(HairTrigger::Builder)
hash == other.hash
other.is_a?(HairTrigger::Builder) && self == other
end

def hash
prepare!
[self.options.hash, self.prepared_actions.hash, self.prepared_where.hash, self.triggers.hash, @compatibility].hash
components.hash
end

def components
[self.options, self.prepared_actions, self.prepared_where, self.triggers, @compatibility]
end

def errors
Expand Down
39 changes: 0 additions & 39 deletions lib/hair_trigger/migration.rb

This file was deleted.

64 changes: 64 additions & 0 deletions lib/hair_trigger/migration_reader.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
require 'ruby_parser'
require 'ruby2ruby'

module HairTrigger
module MigrationReader
class << self
def get_triggers(source, options)
triggers = []
if source.is_a?(String)
# schema.rb contents... because it's auto-generated and we know
# exactly what it will look like, we can safely use a regex
source.scan(/^ create_trigger\(.*?\n end\n\n/m).each do |match|
trigger = instance_eval("generate_" + match.strip)
triggers << trigger if options[:include_manual_triggers] || trigger.options[:generated]
end
else
contents = File.read(source.filename)
return [] unless contents =~ /(create|drop)_trigger/
sexps = RubyParser.new.parse(contents)
# find the migration class
sexps = [sexps] unless sexps[0] == :block
sexps = sexps.detect{ |s| s.is_a?(Sexp) && s[0] == :class && s[1] == source.name.to_sym }.last
# find the block of the up method
sexps = sexps.last if sexps.last.is_a?(Sexp) && sexps.last[0] == :block
sexps = sexps.detect{ |s| s.is_a?(Sexp) && s[0] == :defs && s[1] && s[1][0] == :self && s[2] == :up }.last.last
sexps.each do |sexp|
next unless (method = extract_method_call(sexp)) && [:create_trigger, :drop_trigger].include?(method)
trigger = instance_eval("generate_" + generator.process(sexp))
triggers << trigger if options[:include_manual_triggers] || trigger.options[:generated]
end
end
triggers
rescue
$stderr.puts "Error reading triggers in #{source.filename rescue "schema.rb"}: #{$!}"
end

private
def extract_method_call(exp)
return nil unless exp.is_a?(Array)
if exp[0] == :iter
exp = exp[1] while exp[1].is_a?(Array) && exp[1][0] == :call
end
if exp[0] == :call
exp[2]
end
end

def generate_create_trigger(*arguments)
arguments.unshift({}) if arguments.empty?
arguments.unshift(nil) if arguments.first.is_a?(Hash)
arguments[1][:compatibility] ||= HairTrigger::Builder.base_compatibility
::HairTrigger::Builder.new(*arguments)
end

def generate_drop_trigger(*arguments)
::HairTrigger::Builder.new(arguments[0], {:table => arguments[1], :drop => true})
end

def generator
@generator ||= Ruby2Ruby.new
end
end
end
end
17 changes: 0 additions & 17 deletions lib/hair_trigger/schema.rb

This file was deleted.

19 changes: 19 additions & 0 deletions spec/schema_dumper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def initialize_db(adapter)
}
migration = HairTrigger.generate_migration
ActiveRecord::Migrator.migrate(HairTrigger.migration_path)
HairTrigger.should be_migrations_current
ActiveRecord::Base.connection.triggers.values.grep(/bob_count \+ 1/).size.should eql(0)
ActiveRecord::Base.connection.triggers.values.grep(/bob_count \+ 2/).size.should eql(1)

Expand All @@ -100,6 +101,24 @@ def initialize_db(adapter)
schema_rb4.should_not eql(schema_rb3)
schema_rb4.should eql(schema_rb2)
ActiveRecord::Base.connection.triggers.values.grep(/bob_count \+ 1/).size.should eql(1)

# delete our migrations, it should still dump correctly
FileUtils.rm_rf(Dir.glob('tmp/migrations/*rb'))
ActiveRecord::SchemaDumper.previous_schema = schema_rb4
io = StringIO.new
ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, io)
io.rewind
schema_rb5 = io.read
schema_rb5.should eql(schema_rb4)

# "delete" schema.rb too, now it should have adapter-specific triggers
ActiveRecord::SchemaDumper.previous_schema = nil
io = StringIO.new
ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, io)
io.rewind
schema_rb6 = io.read
schema_rb6.should_not match(/create_trigger\(/)
schema_rb6.should match(/no candidate create_trigger statement could be found, creating an adapter-specific one/)
end
end
end

0 comments on commit d34d091

Please sign in to comment.