diff --git a/lib/schema_plus/active_record/connection_adapters/foreign_key_definition.rb b/lib/schema_plus/active_record/connection_adapters/foreign_key_definition.rb index 3dd1409..c377746 100644 --- a/lib/schema_plus/active_record/connection_adapters/foreign_key_definition.rb +++ b/lib/schema_plus/active_record/connection_adapters/foreign_key_definition.rb @@ -65,7 +65,7 @@ class ForeignKeyDefinition < ::ActiveRecord::ConnectionAdapters::ForeignKeyDefin ACTIONS = { :cascade => "CASCADE", :restrict => "RESTRICT", :set_null => "SET NULL", :set_default => "SET DEFAULT", :no_action => "NO ACTION" }.freeze - def initialize(from_table, to_table, options) + def initialize(from_table, to_table, options) super @from_table = unquote(from_table) @to_table = unquote(to_table) diff --git a/lib/schema_plus/active_record/connection_adapters/schema_statements.rb b/lib/schema_plus/active_record/connection_adapters/schema_statements.rb index de2c518..a026ecd 100644 --- a/lib/schema_plus/active_record/connection_adapters/schema_statements.rb +++ b/lib/schema_plus/active_record/connection_adapters/schema_statements.rb @@ -63,7 +63,7 @@ def self.add_index_exception_handler(connection, table, columns, options, e) #:n raise unless e.message.match(/["']([^"']+)["'].*already exists/) name = $1 existing = connection.indexes(table).find{|i| i.name == name} - attempted = ::ActiveRecord::ConnectionAdapters::IndexDefinition.new(table, columns, options.merge(:name => name)) + attempted = ::ActiveRecord::ConnectionAdapters::IndexDefinition.new(table, columns, options.merge(:name => name)) raise if attempted != existing ::ActiveRecord::Base.logger.warn "[schema_plus] Index name #{name.inspect}' on table #{table.inspect} already exists. Skipping." end diff --git a/lib/schema_plus/active_record/schema_dumper.rb b/lib/schema_plus/active_record/schema_dumper.rb index f1287ca..c4f86c3 100644 --- a/lib/schema_plus/active_record/schema_dumper.rb +++ b/lib/schema_plus/active_record/schema_dumper.rb @@ -39,7 +39,7 @@ def foreign_keys_with_schema_plus(*) def break_fk_cycles #:nodoc: strongly_connected_components.select{|component| component.size > 1}.each do |tables| - table = tables.sort.first + table = tables.sort.last backref_fks = @inline_fks[table].select{|fk| tables.include?(fk.references_table_name)} @inline_fks[table] -= backref_fks @dump_dependencies[table] -= backref_fks.collect(&:references_table_name) @@ -149,7 +149,7 @@ def dump_indexes(table) #:nodoc: dump << " :name => #{index.name.inspect}" dump << ", :unique => true" if index.unique dump << ", :kind => \"#{index.kind}\"" unless index.kind.blank? - unless index.columns.blank? + unless index.columns.blank? dump << ", :case_sensitive => false" unless index.case_sensitive? dump << ", :conditions => #{index.conditions.inspect}" unless index.conditions.blank? index_lengths = index.lengths.compact if index.lengths.is_a?(Array) diff --git a/spec/schema_dumper_spec.rb b/spec/schema_dumper_spec.rb index caa9370..b8b708c 100644 --- a/spec/schema_dumper_spec.rb +++ b/spec/schema_dumper_spec.rb @@ -160,7 +160,7 @@ class ::Comment < ActiveRecord::Base ; end expect(dump_posts).to match(to_regexp(%q{t.index ["user_id"], :name => "custom_name"})) end end - + it "should define unique index" do with_index Post, :user_id, :name => "posts_user_id_index", :unique => true do expect(dump_posts).to match(to_regexp(%q{t.index ["user_id"], :name => "posts_user_id_index", :unique => true})) @@ -284,6 +284,97 @@ class ::Comment < ActiveRecord::Base ; end expect(dump_schema).to match(%r{create_table "users".*foreign_key.*\["commenter_id"\], "users", \["id"\]}m) expect(dump_schema).to match(%r{create_table "users".*foreign_key.*\["user_id"\], "users", \["id"\]}m) end + + context 'with complicated schemas' do + before(:all) do + + SchemaPlus.setup do |config| + config.foreign_keys.auto_create = false + end + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Schema.define do + connection.tables.each do |table| drop_table table, :cascade => true end + + create_table :period_types, force: true do |t| + t.string :name + end + + create_table :grade_systems, force: true do |t| + t.string :name + t.integer :school_id + t.integer :parent_id + t.integer :profile_id + end + + create_table :schools, force: true do |t| + t.string :name + t.integer :default_grade_system_id + end + + create_table :academic_years, force: true do |t| + t.string :name + t.integer :school_id + t.integer :period_type_id + end + + create_table :buildings, force: true do |t| + t.string :name + t.integer :school_id + end + + create_table :publishing_houses, force: true do |t| + t.string :name + end + + create_table :profiles, force: true do |t| + t.integer :school_id + t.integer :publishing_house_id + t.integer :building_id + end + + create_table :class_units, force: true do |t| + t.string :name + t.integer :school_id + t.integer :mentor_id + t.integer :building_id + end + end + end + + class ::AcademicYear < ActiveRecord::Base ; end + class ::Building < ActiveRecord::Base ; end + class ::ClassUnit < ActiveRecord::Base ; end + class ::GradeSystem < ActiveRecord::Base ; end + class ::Profile < ActiveRecord::Base ; end + class ::PublishingHouse < ActiveRecord::Base ; end + class ::PeriodType < ActiveRecord::Base ; end + class ::School < ActiveRecord::Base ; end + + ActiveRecord::Base.connection.add_foreign_key(School.table_name, :default_grade_system_id, GradeSystem.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(GradeSystem.table_name, :school_id, School.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(GradeSystem.table_name, :parent_id, GradeSystem.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(GradeSystem.table_name, :profile_id, Profile.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(Profile.table_name, :building_id, Building.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(Profile.table_name, :school_id, School.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(ClassUnit.table_name, :school_id, School.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(ClassUnit.table_name, :building_id, Building.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(ClassUnit.table_name, :mentor_id, Profile.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(Building.table_name, :school_id, School.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(AcademicYear.table_name, :school_id, School.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(AcademicYear.table_name, :period_type_id, PeriodType.table_name, :id) + ActiveRecord::Base.connection.add_foreign_key(Profile.table_name, :publishing_house_id, PublishingHouse.table_name, :id) + end + + it "should not raise an error" do + expect { dump_schema }.to_not raise_error + end + + it "should dump each constraint after both related tables were defined" do + expect(dump_schema.scan(%r{add_foreign_key}m).count).to eq 1 + expect(dump_schema).to match(%r{create_table "schools".*add_foreign_key\s+"schools".*\["default_grade_system_id"\], "grade_systems", \["id"\]}m) + expect(dump_schema).to match(%r{create_table "grade_systems".*add_foreign_key\s+"schools".*\["default_grade_system_id"\], "grade_systems", \["id"\]}m) + end + end end context 'with enum', :postgresql => :only do