From d707d45db1b7f67d19be03c3d56f06159472a279 Mon Sep 17 00:00:00 2001 From: Michael H Buselli Date: Mon, 10 Nov 2014 15:46:44 -0600 Subject: [PATCH 1/2] Permit symbols for index names --- lib/mini_record/auto_schema.rb | 4 ++-- test/test_mini_record.rb | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/lib/mini_record/auto_schema.rb b/lib/mini_record/auto_schema.rb index ab4eb6b..9e9cf46 100644 --- a/lib/mini_record/auto_schema.rb +++ b/lib/mini_record/auto_schema.rb @@ -324,7 +324,7 @@ class << connection; attr_accessor :table_definition; end unless connection.resp remove_foreign_keys if connection.respond_to?(:foreign_keys) # Remove old index - index_names = indexes.collect{|name,opts| opts[:name] || name } + index_names = indexes.collect{ |name, opts| (opts[:name] || name).to_s } (indexes_in_db.keys - index_names).each do |name| connection.remove_index(table_name, :name => name) end @@ -344,7 +344,7 @@ class << connection; attr_accessor :table_definition; end unless connection.resp # Add indexes indexes.each do |name, options| options = options.dup - index_name = options[:name] || name + index_name = (options[:name] || name).to_s unless connection.indexes(table_name).detect { |i| i.name == index_name } connection.add_index(table_name, options.delete(:column), options) end diff --git a/test/test_mini_record.rb b/test/test_mini_record.rb index 4a09c0f..83c5df7 100644 --- a/test/test_mini_record.rb +++ b/test/test_mini_record.rb @@ -128,9 +128,41 @@ class Foo < ActiveRecord::Base index :customer_id, :unique => true, :name => 'by_customer' belongs_to :customer end + # Run auto_upgrade! once to create table and index. Foo.auto_upgrade! assert_equal 1, Foo.db_indexes.size assert_includes Foo.db_indexes, 'by_customer' + # Run auto_upgrade! again and ensure no statements issued. + Foo.auto_upgrade! + assert_empty Foo.queries + end + + it 'does not add already defined composite indexes' do + class Foo < ActiveRecord::Base + belongs_to :region + belongs_to :customer + add_index [:region_id, :customer_id], :unique => true, :name => 'by_region_and_customer' + end + # Run auto_upgrade! once to create table and index. + Foo.auto_upgrade! + assert_equal 3, Foo.db_indexes.size + assert_includes Foo.db_indexes, 'by_region_and_customer' + # Run auto_upgrade! again and ensure no statements issued. + Foo.auto_upgrade! + assert_empty Foo.queries + end + + it 'supports indexes with symbols for names' do + class Foo < ActiveRecord::Base + col :some_field, :index => {:name => :idx_for_some_field} + end + # Run auto_upgrade! once to create table and index. + Foo.auto_upgrade! + assert_equal 1, Foo.db_indexes.size + assert_includes Foo.db_indexes, 'idx_for_some_field' + # Run auto_upgrade! again and ensure no statements issued. + Foo.auto_upgrade! + assert_empty Foo.queries end it 'works with STI' do From 793e08d78a8295a7ab58ea661181f4841eed14c1 Mon Sep 17 00:00:00 2001 From: Michael H Buselli Date: Wed, 12 Nov 2014 11:12:38 -0600 Subject: [PATCH 2/2] Add dry run and more verbose logging Dry run and verbose logging are added together because without dry run, the verbose logging is technically unnecessary as all schema changes are logged via the logged SQL they generate. Likewise, without verbose logging, a dry run cannot tell you want it will do on a live run. --- lib/mini_record/auto_schema.rb | 93 ++++++++++++++++++++++------------ test/helper.rb | 8 ++- test/test_mini_record.rb | 43 ++++++++++++++-- 3 files changed, 105 insertions(+), 39 deletions(-) diff --git a/lib/mini_record/auto_schema.rb b/lib/mini_record/auto_schema.rb index 7507e03..04c5351 100644 --- a/lib/mini_record/auto_schema.rb +++ b/lib/mini_record/auto_schema.rb @@ -165,11 +165,14 @@ def connection? false end - def clear_tables! + def clear_tables!(dry_run = false) return unless MiniRecord.configuration.destructive == true (connection.tables - schema_tables).each do |name| - connection.drop_table(name) - schema_tables.delete(name) + logger.debug "[MiniRecord] Dropping table #{name}" + unless dry_run + connection.drop_table(name) + schema_tables.delete(name) + end end end @@ -180,13 +183,14 @@ def foreign_keys end # Remove foreign keys for indexes with :foreign=>false option - def remove_foreign_keys + def remove_foreign_keys(dry_run) return unless MiniRecord.configuration.destructive == true indexes.each do |name, options| if options[:foreign]==false foreign_key = foreign_keys.detect { |fk| fk.options[:column] == options[:column].to_s } if foreign_key - connection.remove_foreign_key(table_name, :name => foreign_key.options[:name]) + logger.debug "[MiniRecord] Removing Foreign Key #{foreign_key.options[:name]} on table #{table_name}" + connection.remove_foreign_key(table_name, :name => foreign_key.options[:name]) unless dry_run foreign_keys.delete(foreign_key) end end @@ -194,33 +198,42 @@ def remove_foreign_keys end # Add foreign keys for indexes with :foreign=>true option, if the key doesn't exists - def add_foreign_keys + def add_foreign_keys(dry_run) indexes.each do |name, options| if options[:foreign] column = options[:column].to_s unless foreign_keys.detect { |fk| fk[:options][:column] == column } to_table = reflect_on_all_associations.detect { |a| a.foreign_key.to_s==column }.table_name - connection.add_foreign_key(table_name, to_table, options) + logger.debug "[MiniRecord] Adding Foreign Key on #{table_name} to #{to_table}" + connection.add_foreign_key(table_name, to_table, options) unless dry_run foreign_keys << { :options=> { :column=>column } } end end end end - def auto_upgrade! + # dry-run + def auto_upgrade_dry + auto_upgrade!(true) + end + + def auto_upgrade!(dry_run = false) return unless connection? return if respond_to?(:abstract_class?) && abstract_class? if self == ActiveRecord::Base - descendants.each(&:auto_upgrade!) - clear_tables! + descendants.each { |model| model.auto_upgrade!(dry_run) } + clear_tables!(dry_run) else # If table doesn't exist, create it unless connection.tables.include?(table_name) class << connection; attr_accessor :table_definition; end unless connection.respond_to?(:table_definition=) - connection.table_definition = table_definition - connection.create_table(table_name, *create_table_options) - connection.table_definition = init_table_definition(connection) + logger.debug "[MiniRecord] Creating Table #{table_name}" + unless dry_run + connection.table_definition = table_definition + connection.create_table(table_name, *create_table_options) + connection.table_definition = init_table_definition(connection) + end end # Add this to our schema tables @@ -249,13 +262,17 @@ class << connection; attr_accessor :table_definition; end unless connection.resp unless connection.tables.include?(table.to_s) foreign_key = association.options[:foreign_key] || association.foreign_key association_foreign_key = association.options[:association_foreign_key] || association.association_foreign_key - connection.create_table(table, :id => false) do |t| - t.integer foreign_key - t.integer association_foreign_key + logger.debug "[MiniRecord] Creating Join Table #{table} with keys #{foreign_key} and #{association_foreign_key}" + unless dry_run + connection.create_table(table, :id => false) do |t| + t.integer foreign_key + t.integer association_foreign_key + end end index_name = connection.index_name(table, :column => [foreign_key, association_foreign_key]) index_name = index_name[0...connection.index_name_length] if index_name.length > connection.index_name_length - connection.add_index table, [foreign_key, association_foreign_key], :name => index_name, :unique => true + logger.debug "[MiniRecord] Creating Join Table Index #{index_name} (#{foreign_key}, #{association_foreign_key}) on #{table}" + connection.add_index table, [foreign_key, association_foreign_key], :name => index_name, :unique => true unless dry_run end # Add join table to our schema tables schema_tables << table unless schema_tables.include?(table) @@ -270,14 +287,15 @@ class << connection; attr_accessor :table_definition; end unless connection.resp end # Group Destructive Actions - if MiniRecord.configuration.destructive == true + if MiniRecord.configuration.destructive == true and connection.tables.include?(table_name) # Rename fields rename_fields.each do |old_name, new_name| old_column = fields_in_db[old_name.to_s] new_column = fields_in_db[new_name.to_s] if old_column && !new_column - connection.rename_column(table_name, old_column.name, new_name) + logger.debug "[MiniRecord] Renaming column #{table_name}.#{old_column.name} to #{new_name}" + connection.rename_column(table_name, old_column.name, new_name) unless dry_run end end @@ -285,7 +303,8 @@ class << connection; attr_accessor :table_definition; end unless connection.resp columns_to_delete = fields_in_db.keys - fields.keys & fields_in_db.keys columns_to_delete.each do |field| column = fields_in_db[field] - connection.remove_column table_name, column.name + logger.debug "[MiniRecord] Removing column #{table_name}.#{column.name}" + connection.remove_column table_name, column.name unless dry_run end # Change attributes of existent columns @@ -324,28 +343,35 @@ class << connection; attr_accessor :table_definition; end unless connection.resp # Change the column if applicable new_type = fields[field].type.to_sym - connection.change_column table_name, field, new_type, new_attr if changed + if changed + logger.debug "[MiniRecord] Changing column #{table_name}.#{field} to new type #{new_type}" + connection.change_column table_name, field, new_type, new_attr unless dry_run + end end end - remove_foreign_keys if connection.respond_to?(:foreign_keys) + remove_foreign_keys(dry_run) if connection.respond_to?(:foreign_keys) # Remove old index index_names = indexes.collect{|name,opts| opts[:name] || name } (indexes_in_db.keys - index_names).each do |name| - connection.remove_index(table_name, :name => name) + logger.debug "[MiniRecord] Removing index #{name} on #{table_name}" + connection.remove_index(table_name, :name => name) unless dry_run end end - # Add fields to db new to schema - columns_to_add = fields.keys - fields_in_db.keys - columns_to_add.each do |field| - column = fields[field] - options = {:limit => column.limit, :precision => column.precision, :scale => column.scale} - options[:default] = column.default unless column.default.nil? - options[:null] = column.null unless column.null.nil? - connection.add_column table_name, column.name, column.type.to_sym, options + if connection.tables.include?(table_name) + # Add fields to db new to schema + columns_to_add = fields.keys - fields_in_db.keys + columns_to_add.each do |field| + column = fields[field] + options = {:limit => column.limit, :precision => column.precision, :scale => column.scale} + options[:default] = column.default unless column.default.nil? + options[:null] = column.null unless column.null.nil? + logger.debug "[MiniRecord] Adding column #{table_name}.#{column.name}" + connection.add_column table_name, column.name, column.type.to_sym, options unless dry_run + end end # Add indexes @@ -353,11 +379,12 @@ class << connection; attr_accessor :table_definition; end unless connection.resp options = options.dup index_name = options[:name] || name unless connection.indexes(table_name).detect { |i| i.name == index_name } - connection.add_index(table_name, options.delete(:column), options) + logger.debug "[MiniRecord] Adding index #{index_name} #{options[:column].inspect} on #{table_name}" + connection.add_index(table_name, options.delete(:column), options) unless dry_run end end - add_foreign_keys if connection.respond_to?(:foreign_keys) + add_foreign_keys(dry_run) if connection.respond_to?(:foreign_keys) # Reload column information reset_column_information diff --git a/test/helper.rb b/test/helper.rb index 4cd3e5f..1983420 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -37,7 +37,13 @@ def queries(pragma=false) ActiveRecord::Base.logs.string.gsub(/\e\[[\d;]+m/, '').lines.reject { |l| !pragma && l =~ /pragma/i }.join("\n") end - def auto_upgrade! + def auto_upgrade!(*args) + ActiveRecord::Base.logs = StringIO.new + ActiveRecord::Base.logger = ActiveSupport::BufferedLogger.new(ActiveRecord::Base.logs) + silence_stream(STDERR) { super } + end + + def auto_upgrade_dry ActiveRecord::Base.logs = StringIO.new ActiveRecord::Base.logger = ActiveSupport::BufferedLogger.new(ActiveRecord::Base.logs) silence_stream(STDERR) { super } diff --git a/test/test_mini_record.rb b/test/test_mini_record.rb index 77832ab..c34ab76 100644 --- a/test/test_mini_record.rb +++ b/test/test_mini_record.rb @@ -2,13 +2,20 @@ describe MiniRecord do - before do - ActiveRecord::Base.clear_reloadable_connections! - ActiveRecord::Base.clear_cache! - ActiveRecord::Base.clear_active_connections! - conn.tables.each { |table| silence_stream(STDERR) { conn.execute "DROP TABLE IF EXISTS #{table}" } } + def clear_active_record!(options = {}) + unless options[:keep_tables] + ActiveRecord::Base.clear_reloadable_connections! + ActiveRecord::Base.clear_cache! + ActiveRecord::Base.clear_active_connections! + conn.tables.each { |table| silence_stream(STDERR) { conn.execute "DROP TABLE IF EXISTS #{table}" } } + end + ActiveRecord::Base.descendants.each { |klass| Object.send(:remove_const, klass.to_s) if Object.const_defined?(klass.to_s) } ActiveSupport::DescendantsTracker.direct_descendants(ActiveRecord::Base).clear + end + + before do + clear_active_record! load File.expand_path('../models.rb', __FILE__) ActiveRecord::Base.auto_upgrade! MiniRecord.reset_configuration! @@ -895,4 +902,30 @@ class Foo < ActiveRecord::Base Foo.auto_upgrade! rescue nil # eat the exception from invalid options assert_match /CREATE TABLE.* extra options\Z/, Foo.queries end + + it 'can do a dry run' do + class Foo < ActiveRecord::Base + end + + ActiveRecord::Base.auto_upgrade_dry + refute_match /\bcreate\b/i, Foo.queries + refute_match /\balter\b/i, Foo.queries + + ActiveRecord::Base.auto_upgrade! + assert_match /\bcreate\b/i, Foo.queries + refute_match /\balter\b/i, Foo.queries + + clear_active_record!(:keep_tables => true) + class Foo < ActiveRecord::Base + property :new_field, :index => true + end + + ActiveRecord::Base.auto_upgrade_dry + refute_match /\bcreate\b/i, Foo.queries + refute_match /\balter\b/i, Foo.queries + + ActiveRecord::Base.auto_upgrade! + assert_match /\bcreate\b/i, Foo.queries + assert_match /\balter\b/i, Foo.queries + end end